[PATCH] spi: davinci: remove empty function davinci_spi_cleanup
Remove empty function davinci_spi_cleanup().
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/spi/spi-davinci.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index d493507..134fb6e 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -436,10 +436,6 @@ static int davinci_spi_setup(struct spi_device *spi)
return retval;
}
-static void davinci_spi_cleanup(struct spi_device *spi)
-{
-}
-
static int davinci_spi_check_error(struct davinci_spi *dspi, int int_status)
{
struct device *sdev = dspi-bitbang.master-dev.parent;
@@ -951,7 +947,6 @@ static int davinci_spi_probe(struct platform_device *pdev)
master-num_chipselect = pdata-num_chipselect;
master-bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16);
master-setup = davinci_spi_setup;
- master-cleanup = davinci_spi_cleanup;
dspi-bitbang.chipselect = davinci_spi_chipselect;
dspi-bitbang.setup_transfer = davinci_spi_setup_transfer;
--
1.9.1
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RESEND PATCH v3] spi: davinci: add support for adding delay between word's transmissions
From: Murali Karicheri m-kariche...@ti.com
This patch adds ability to configure delay between transmission of
words over SPI bus if it's required by SPI slave devices.
New optional SPI slave property:
- ti,spi-word-delay : delay between transmission of words
(SPIFMTn.WDELAY, SPIDAT1.WDEL)
Signed-off-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Rebased on top of:
spi.git/for-next +
[PATCH] spi: davinci: remove empty function davinci_spi_cleanup
http://www.spinics.net/lists/arm-kernel/msg362811.html
v3:
http://www.spinics.net/lists/devicetree/msg49141.html
v2:
http://www.spinics.net/lists/devicetree/msg48514.html
v1:
http://www.spinics.net/lists/linux-spi/msg01609.html
.../devicetree/bindings/spi/spi-davinci.txt| 30
drivers/spi/spi-davinci.c | 55 +++---
2 files changed, 78 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt
b/Documentation/devicetree/bindings/spi/spi-davinci.txt
index f80887b..12ecfe9 100644
--- a/Documentation/devicetree/bindings/spi/spi-davinci.txt
+++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
@@ -1,5 +1,10 @@
Davinci SPI controller device bindings
+Links on DM:
+Keystone 2 - http://www.ti.com/lit/ug/sprugp2a/sprugp2a.pdf
+dm644x - http://www.ti.com/lit/ug/sprue32a/sprue32a.pdf
+OMAP-L138/da830 - http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf
+
Required properties:
- #address-cells: number of cells required to define a chip select
address on the SPI bus. Should be set to 1.
@@ -24,6 +29,30 @@ Optional:
cs-gpios = 0, 0, 0, gpio1 30 0, gpio1 31 0;
where first three are internal CS and last two are GPIO CS.
+Optional properties for slave devices:
+SPI slave nodes can contain the following properties.
+Not all SPI Peripherals from Texas Instruments support this.
+Please check SPI peripheral documentation for a device before using these.
+
+- ti,spi-wdelay : delay between transmission of words
+ (SPIFMTn.WDELAY, SPIDAT1.WDEL) must be specified in number of SPI module
+ clock periods.
+
+ delay = WDELAY * SPI_module_clock_period + 2 * SPI_module_clock_period
+
+Below is timing diagram which shows functional meaning of
+ti,spi-wdelay parameter.
+
+ +-+ +-+ +-+ +-+ +-+ +-+ +-+ +-+
+SPI_CLK | | | | | | | | | | | | | | | |
+ +--+ +-+ +-+ +-+ +-+ +---+ +-+ +-+ +-
+
+SPI_SOMI/SIMO+-+ +---
+ +--+ word1 +---+word2
+ +-+ +---
+ WDELAY
+--
+
Example of a NOR flash slave device (n25q032) connected to DaVinci
SPI controller device over the SPI bus.
@@ -43,6 +72,7 @@ spi0:spi@20BF {
compatible = st,m25p32;
spi-max-frequency = 2500;
reg = 0;
+ ti,spi-wdelay = 8;
partition@0 {
label = u-boot-spl;
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 134fb6e..ada3891 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -65,6 +65,7 @@
/* SPIDAT1 (upper 16 bit defines) */
#define SPIDAT1_CSHOLD_MASKBIT(12)
+#define SPIDAT1_WDEL BIT(10)
/* SPIGCR1 */
#define SPIGCR1_CLKMOD_MASKBIT(1)
@@ -209,6 +210,7 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
{
struct davinci_spi *dspi;
struct davinci_spi_platform_data *pdata;
+ struct davinci_spi_config *spicfg = spi-controller_data;
u8 chip_sel = spi-chip_select;
u16 spidat1 = CS_DEFAULT;
bool gpio_chipsel = false;
@@ -223,6 +225,10 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
gpio = spi-cs_gpio;
}
+ /* program delay transfers if tx_delay is non zero */
+ if (spicfg-wdelay)
+ spidat1 |= SPIDAT1_WDEL;
+
/*
* Board specific chip select logic decides the polarity and cs
* line for the controller
@@ -237,9 +243,9 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
spidat1 |= SPIDAT1_CSHOLD_MASK;
spidat1 = ~(0x1 chip_sel);
}
-
- iowrite16(spidat1, dspi-base + SPIDAT1 + 2);
}
+
+ iowrite16(spidat1, dspi-base + SPIDAT1 + 2);
}
/**
@@ -285,7 +291,7 @@ static int davinci_spi_setup_transfer(struct spi_device
*spi,
int prescale;
dspi = spi_master_get_devdata(spi-master);
- spicfg = (struct davinci_spi_config *)spi-controller_data;
+ spicfg = spi-controller_data;
if (!spicfg
Re: [PATCH v3] spi: davinci: add support for adding delay between word's transmissions
On 09/13/2014 07:06 PM, Mark Brown wrote: On Fri, Sep 12, 2014 at 06:48:12PM +0300, Grygorii Strashko wrote: From: Murali Karicheri m-kariche...@ti.com This patch adds ability to configure delay between transmission of words over SPI bus if it's required by SPI slave devices. This is fine but it doesn't appear to apply against current code. Can you please check and resend? This one should be applied without conflicts on top of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git branch: for-next since you've applied the patch: [PATCH] spi: davinci: request cs_gpio's from probe http://www.spinics.net/lists/linux-spi/msg01811.html I have to mention about this dependency, sorry for that. Regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] spi: davinci: request cs_gpio's from probe
On 09/13/2014 07:28 PM, Mark Brown wrote:
On Fri, Sep 12, 2014 at 05:54:00PM +0300, Grygorii Strashko wrote:
static void davinci_spi_cleanup(struct spi_device *spi)
{
-if (spi-cs_gpio = 0)
-gpio_free(spi-cs_gpio);
}
This function is now empty so should be removed. I've applied for now
but please send a followup fixing this.
The function davinci_spi_cleanup() will be reused by following patch
[PATCH v3] spi: davinci: add support for adding delay between word's
transmissions
http://www.spinics.net/lists/devicetree/msg49141.html
So, in broonie/spi.git/for-next it will not be empty.
Again, I've missed description of this dependency, sorry for that.
Regards,
-grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH] spi: davinci: request cs_gpio's from probe
Now CS GPIOs are requested from struct spi_master.setup() callback
and that causes failures when Client SPI device is getting accessed
through SPIDEV driver. The failure happens, because .setup() callback
may be called many times from IOCTL handler and when it's called
second time gpio_request() will fail and return -EBUSY.
Hence, fix it by moving CS GPIOs requesting code in .probe().
Reported-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Tested using spidev_test utility.
drivers/spi/spi-davinci.c | 34 +-
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 48f1d26..d493507 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -397,24 +397,21 @@ static int davinci_spi_setup(struct spi_device *spi)
struct spi_master *master = spi-master;
struct device_node *np = spi-dev.of_node;
bool internal_cs = true;
- unsigned long flags = GPIOF_DIR_OUT;
dspi = spi_master_get_devdata(spi-master);
pdata = dspi-pdata;
- flags |= (spi-mode SPI_CS_HIGH) ? GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
-
if (!(spi-mode SPI_NO_CS)) {
if (np (master-cs_gpios != NULL) (spi-cs_gpio = 0)) {
- retval = gpio_request_one(spi-cs_gpio,
- flags, dev_name(spi-dev));
+ retval = gpio_direction_output(
+ spi-cs_gpio, !(spi-mode SPI_CS_HIGH));
internal_cs = false;
} else if (pdata-chip_sel
spi-chip_select pdata-num_chipselect
pdata-chip_sel[spi-chip_select] != SPI_INTERN_CS) {
spi-cs_gpio = pdata-chip_sel[spi-chip_select];
- retval = gpio_request_one(spi-cs_gpio,
- flags, dev_name(spi-dev));
+ retval = gpio_direction_output(
+ spi-cs_gpio, !(spi-mode SPI_CS_HIGH));
internal_cs = false;
}
@@ -441,8 +438,6 @@ static int davinci_spi_setup(struct spi_device *spi)
static void davinci_spi_cleanup(struct spi_device *spi)
{
- if (spi-cs_gpio = 0)
- gpio_free(spi-cs_gpio);
}
static int davinci_spi_check_error(struct davinci_spi *dspi, int int_status)
@@ -967,6 +962,27 @@ static int davinci_spi_probe(struct platform_device *pdev)
if (dspi-version == SPI_VERSION_2)
dspi-bitbang.flags |= SPI_READY;
+ if (pdev-dev.of_node) {
+ int i;
+
+ for (i = 0; i pdata-num_chipselect; i++) {
+ int cs_gpio = of_get_named_gpio(pdev-dev.of_node,
+ cs-gpios, i);
+
+ if (cs_gpio == -EPROBE_DEFER) {
+ ret = cs_gpio;
+ goto free_clk;
+ }
+
+ if (gpio_is_valid(cs_gpio)) {
+ ret = devm_gpio_request(pdev-dev, cs_gpio,
+ dev_name(pdev-dev));
+ if (ret)
+ goto free_clk;
+ }
+ }
+ }
+
r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
if (r)
dma_rx_chan = r-start;
--
1.9.1
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v3] spi: davinci: add support for adding delay between word's transmissions
From: Murali Karicheri m-kariche...@ti.com
This patch adds ability to configure delay between transmission of
words over SPI bus if it's required by SPI slave devices.
New optional SPI slave's property:
- ti,spi-word-delay : delay between transmission of words
(SPIFMTn.WDELAY, SPIDAT1.WDEL)
Signed-off-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Changes in v3:
- removed C2TDELAY, T2CDELAY parameters
v2:
http://www.spinics.net/lists/devicetree/msg48514.html
v1:
http://www.spinics.net/lists/linux-spi/msg01609.html
.../devicetree/bindings/spi/spi-davinci.txt| 30 +
drivers/spi/spi-davinci.c | 50 +++---
2 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt
b/Documentation/devicetree/bindings/spi/spi-davinci.txt
index f80887b..12ecfe9 100644
--- a/Documentation/devicetree/bindings/spi/spi-davinci.txt
+++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
@@ -1,5 +1,10 @@
Davinci SPI controller device bindings
+Links on DM:
+Keystone 2 - http://www.ti.com/lit/ug/sprugp2a/sprugp2a.pdf
+dm644x - http://www.ti.com/lit/ug/sprue32a/sprue32a.pdf
+OMAP-L138/da830 - http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf
+
Required properties:
- #address-cells: number of cells required to define a chip select
address on the SPI bus. Should be set to 1.
@@ -24,6 +29,30 @@ Optional:
cs-gpios = 0, 0, 0, gpio1 30 0, gpio1 31 0;
where first three are internal CS and last two are GPIO CS.
+Optional properties for slave devices:
+SPI slave nodes can contain the following properties.
+Not all SPI Peripherals from Texas Instruments support this.
+Please check SPI peripheral documentation for a device before using these.
+
+- ti,spi-wdelay : delay between transmission of words
+ (SPIFMTn.WDELAY, SPIDAT1.WDEL) must be specified in number of SPI module
+ clock periods.
+
+ delay = WDELAY * SPI_module_clock_period + 2 * SPI_module_clock_period
+
+Below is timing diagram which shows functional meaning of
+ti,spi-wdelay parameter.
+
+ +-+ +-+ +-+ +-+ +-+ +-+ +-+ +-+
+SPI_CLK | | | | | | | | | | | | | | | |
+ +--+ +-+ +-+ +-+ +-+ +---+ +-+ +-+ +-
+
+SPI_SOMI/SIMO+-+ +---
+ +--+ word1 +---+word2
+ +-+ +---
+ WDELAY
+--
+
Example of a NOR flash slave device (n25q032) connected to DaVinci
SPI controller device over the SPI bus.
@@ -43,6 +72,7 @@ spi0:spi@20BF {
compatible = st,m25p32;
spi-max-frequency = 2500;
reg = 0;
+ ti,spi-wdelay = 8;
partition@0 {
label = u-boot-spl;
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index d493507..ada3891 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -65,6 +65,7 @@
/* SPIDAT1 (upper 16 bit defines) */
#define SPIDAT1_CSHOLD_MASKBIT(12)
+#define SPIDAT1_WDEL BIT(10)
/* SPIGCR1 */
#define SPIGCR1_CLKMOD_MASKBIT(1)
@@ -209,6 +210,7 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
{
struct davinci_spi *dspi;
struct davinci_spi_platform_data *pdata;
+ struct davinci_spi_config *spicfg = spi-controller_data;
u8 chip_sel = spi-chip_select;
u16 spidat1 = CS_DEFAULT;
bool gpio_chipsel = false;
@@ -223,6 +225,10 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
gpio = spi-cs_gpio;
}
+ /* program delay transfers if tx_delay is non zero */
+ if (spicfg-wdelay)
+ spidat1 |= SPIDAT1_WDEL;
+
/*
* Board specific chip select logic decides the polarity and cs
* line for the controller
@@ -237,9 +243,9 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
spidat1 |= SPIDAT1_CSHOLD_MASK;
spidat1 = ~(0x1 chip_sel);
}
-
- iowrite16(spidat1, dspi-base + SPIDAT1 + 2);
}
+
+ iowrite16(spidat1, dspi-base + SPIDAT1 + 2);
}
/**
@@ -285,7 +291,7 @@ static int davinci_spi_setup_transfer(struct spi_device
*spi,
int prescale;
dspi = spi_master_get_devdata(spi-master);
- spicfg = (struct davinci_spi_config *)spi-controller_data;
+ spicfg = spi-controller_data;
if (!spicfg)
spicfg = davinci_spi_default_cfg;
@@ -333,6 +339,14 @@ static int davinci_spi_setup_transfer(struct spi_device
*spi,
spifmt
Re: [PATCH v2] spi: davinci: add support for adding delay between word's transmission
Hi Mark, On 09/09/2014 10:58 PM, Mark Brown wrote: On Tue, Sep 09, 2014 at 02:20:27PM -0400, Murali Karicheri wrote: On 09/09/2014 01:20 PM, Mark Brown wrote: If these delays are not related to chip select then the documentation needs to be fixed to not refer to chip select. Ok. So what I understand is the issue is not having the right description to indicate that these parameters are delays associated with tranmission of successive words on the wire. Personally I like these description match with what is described in the device spec/ user guide and what is described above match with that. However we could add additional description as below to to make it more explicit. spi-c2t-delay - delay after CS is asserted before output bits on wire spi-t2c-delay - delay after tramission of bits and before deasserting CS Do you think this will help? No, that doesn't help at all. You keep claiming that these parameters are related to the intervals between words but then providing descriptions of these parameters which relate to intervals around changes in /CS which is patently not something connected with the gaps between words. Either your description of parameters which do not relate to /CS should not need to mention /CS or the parameters are in fact related to /CS, both can't simultaneously be true. This is what we have in DM (http://www.ti.com/lit/ug/sprugp2a/sprugp2a.pdf): = 2.5.1.3 Automatic Delay Between Transfers The SPI master can automatically insert a delay of between 2 and 65 SPI module clock cycles between transmissions. This delay is controlled by the WDELAY field in the SPI data format register n (SPIFMTn) and is enabled by setting the WDEL bit in the SPI transmit data register (SPIDAT1) to 1. The WDELAY period begins when the T2EDELAY period terminates (if T2E delay period is enabled) or when the T2CDELAY period terminates (if T2E delay period was disabled and T2C delay period was enabled) or when the master deasserts SPISCS[n] (if T2E and T2C delay periods are disabled). If a transfer is initiated by writing a 32-bit value to SPIDAT1, then the new values of SPIDAT1.WDEL and SPIFMTn.WDELAY are used; otherwise, the old values of SPIDAT1.WDEL and SPIFMTn.WDELAY are used. The WDELAY delay period is specified by: 2.5.1.4 Chip Select Hold Option There are slave devices available that require the chip select signal to be held continuously active during several consecutive data word transfers. Other slave devices require the chip select signal to be deactivated between consecutive data word transfers. The SPI can support both types of slave devices. The CSHOLD bit in the SPI transmit data register (SPIDAT1) selects between the two options. If the chip select hold option is enabled, the chip select will not toggle between two consecutive accesses. Therefore, the SPIDELAY.T2CDELAY of the first transfer and the SPIDELAY.C2TDELAY of the second transfer will not be applied. However, the wait delay could still be applied between the two transactions, if the WDEL bit in SPIDAT1 is set to 1. When the CSHOLD bit is 0, during the data transmission, the value of the chip select number field (CSNR[n:0]) in the SPIDAT1 register is put on the chip select SPISCS[n] to SPISCS[0] pins. When the transmission finishes, the chip select default pattern (CSDEF[n:0]) is put on the SPISCS[n] to SPISCS[0] pins. = And according to above the final delay between consecutive data word transfers can be formed as sum of T2CDELAY WDELAY, C2TDELAY and can include CS toggling (exactly as shown on diagram). I'm going to drop spi-c2t-delay/spi-t2c-delay properties, because CSHOLD is always set now by Davinci SPI driver, so these delays will not be applied (this feature can be enabled later). But WDELAY is important for us as we have issue with custom SPI device (FPGA based). So, next version will only have ti,spi-word-delay. Is it okay for you? Best regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2] spi: davinci: add support for adding delay between word's transmission
From: Murali Karicheri m-kariche...@ti.com
This patch adds ability to configure delay between transmission of
words over SPI bus if it's required by SPI slave devices.
New optional SPI slave properties:
- ti,spi-word-delay : delay between transmission of words
(SPIFMTn.WDELAY, SPIDAT1.WDEL)
- ti,spi-c2t-delay: Chip-select-active-to-transmit-start delay
(SPIDELAY.C2TDELAY)
- ti,spi-t2c-delay: Transmit-end-to-chip-select-inactive delay
(SPIDELAY.T2CDELAY)
- ti,spi-disable-cstimer: disable CS timer (SPIFMTn.DISCSTIMERS).
If WDELAY, C2TDELAY, T2CDELAY are configured then delay
between transmission of words will be added as following:
wdelay = T2CDELAY + WDELAY + C2TDELAY.
If ti,spi-disable-cstimer is configured:
wdelay = WDELAY.
Signed-off-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Changes in v2:
- reduced number of new parameters
- bindings updated
Link on v1:
http://www.spinics.net/lists/linux-spi/msg01609.html
.../devicetree/bindings/spi/spi-davinci.txt| 50 +
drivers/spi/spi-davinci.c | 62 +++---
2 files changed, 105 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt
b/Documentation/devicetree/bindings/spi/spi-davinci.txt
index f80887b..1c336d4 100644
--- a/Documentation/devicetree/bindings/spi/spi-davinci.txt
+++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
@@ -24,6 +24,54 @@ Optional:
cs-gpios = 0, 0, 0, gpio1 30 0, gpio1 31 0;
where first three are internal CS and last two are GPIO CS.
+Optional properties for slave devices:
+SPI slave nodes can contain the following properties.
+Not all SPI Peripherals from Texas Instruments support this.
+Please check SPI peripheral documentation for a device before using these.
+
+- ti,spi-word-delay : delay between transmission of words
+ (SPIFMTn.WDELAY, SPIDAT1.WDEL) must be specified in number of SPI module
+ clock periods.
+
+ delay = WDELAY * SPI_module_clock_period + 2 * SPI_module_clock_period
+
+- ti,spi-c2t-delay: Chip-select-active-to-transmit-start delay
+ (SPIDELAY.C2TDELAY) must be specified in number of SPI module
+ clock periods. if 0 - delay is not applied.
+
+ delay = (C2TDELAY + 2) * SPI_module_clock_period
+
+- ti,spi-t2c-delay: Transmit-end-to-chip-select-inactive delay
+ (SPIDELAY.T2CDELAY) must be specified in number of SPI module
+ clock periods. if 0 - delay is not applied.
+
+ delay = (T2CDELAY + 1) * SPI_module_clock_period
+
+- ti,spi-disable-cstimer: disable CS timer (SPIFMTn.DISCSTIMERS). Boolean
+ property that the C2TDELAY and T2CDELAY timers should be disabled.
+
+Below is timing diagram which shows functional meaning of
+ti,spi-word-delay, ti,spi-c2t-delay and ti,spi-t2c-delay parameters.
+
+ +-+ +-+ +-+ +-+ +-+ +-+ +-+ +-+
+SPI_CLK | | | | | | | | | | | | | | | |
+ +--+ +-+ +-+ +-+ +-+ +---+ +-+ +-+ +-
+
+SPI_SOMI/SIMO+-+ +---
+ +--+ word1 +---+word2
+ +-+ +---
+ WDELAY
+ -
++ +
+SPI_CS | |
+ +++---+
+ || |
+ +-+-++ +-+
+ | | || | |
+ + + +| + +
+--- -- ---
+ C2TDELAY T2CDELAYC2TDELAY
+
Example of a NOR flash slave device (n25q032) connected to DaVinci
SPI controller device over the SPI bus.
@@ -43,6 +91,8 @@ spi0:spi@20BF {
compatible = st,m25p32;
spi-max-frequency = 2500;
reg = 0;
+ ti,spi-c2t-delay = 8;
+ ti,spi-t2c-delay = 8;
partition@0 {
label = u-boot-spl;
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 48f1d26..ccadc95 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -65,6 +65,7 @@
/* SPIDAT1 (upper 16 bit defines) */
#define SPIDAT1_CSHOLD_MASKBIT(12)
+#define SPIDAT1_WDEL BIT(10)
/* SPIGCR1 */
#define SPIGCR1_CLKMOD_MASKBIT(1)
@@ -209,6 +210,7 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
{
struct davinci_spi *dspi;
struct davinci_spi_platform_data *pdata;
+ struct davinci_spi_config *spicfg = spi-controller_data
Re: [PATCH 2/2] spi: davinci: support adding delay between transmission
Hi Mark, On 09/06/2014 05:31 PM, Mark Brown wrote: On Fri, Sep 05, 2014 at 05:21:56PM +0300, Grygorii Strashko wrote: I think we have some misunderstanding here :( 1) All new properties a optional and should be specified for SPI Slave devices 2) Seems we are talking using different terms: - you referring to the term transfers - sequence of packets. Each packet is one transfer (array of words). - while these new properties affect on transmissions - sequence of words. Each word is one transmission. That's *very* unusual terminology which doesn't match my expectations at all. Please describe words as words, that'll be much more obvious. These terms are used in DM/TRM :( I'll split this patch and first introduce WDELAY, C2TDELAY, T2CDELAY (with updated documentation). The only question is - Should they be somehow common or specific for spi-davinci? Also, below is additional information about properties which are used in 5-pin mode (SPI_READY) to improve error detection [OMAP-L138/da830 - http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf]: This is a *whole* other thing, please split these out and work on this separately. The client device is going to need to be doing the same thing here so implementing this as a local option in the controller driver isn't the best way forwards. SPIFMTn[23].PARPOL - Parity polarity: even or odd. PARPOL can be modified in privilege mode only. 0 An even parity flag is added at the end of the transmit data stream. 1 An odd parity flag is added at the end of the transmit data stream. Why would these be specified in DT and not with runtime flags enabled by the device? It looks like they affect the data stream generated by the controller so the client device needs to know about them; I'd expect that it's device driver would be controlling when they are enabled if the controller supports them. Could you clarify, pls - Do you mean that struct spi_device.mode and common SPI DT bindings should be extended to support this? Yes, if they aren't something that's purely internal to the device they need to be generic so that both devices can be configured appropriately. Regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/2] spi: davinci: support adding delay between transmission
Hi Mark, I'm very sorry for delayed replay. On 08/22/2014 06:06 PM, Mark Brown wrote: On Fri, Aug 22, 2014 at 04:33:09PM +0300, Grygorii Strashko wrote: On 08/21/2014 09:20 PM, Mark Brown wrote: On Thu, Aug 21, 2014 at 06:25:06PM +0300, Grygorii Strashko wrote: +- ti,davinci-spi-wdelay : delay between transmissions. I don't understand why this is here - there is already standard support in the SPI core for client drivers specifying inter-transfer delays. If there is a need to provide platform configuration for this in addition to that then it should also be a standard property, there is nothing device specific about these. Sorry, may be I've missed smth, because I was not able to find such common property in SPI bindings document and code. Could you point me on, pls? It's not a property, it's what the delay_usecs in the transfer does. There is no obvious reason to apply something like this unconditionally on every transfer in DT - either it's needed only at particular points in the transfer for device reasons (in which case the device driver needs to know about it) or we need some sort of association with what the device is doing since the way the client driver splits up transfers is entirely up to it. I think we have some misunderstanding here :( 1) All new properties a optional and should be specified for SPI Slave devices 2) Seems we are talking using different terms: - you referring to the term transfers - sequence of packets. Each packet is one transfer (array of words). - while these new properties affect on transmissions - sequence of words. Each word is one transmission. Below is timing diagram which shows, in general, how these new parameters affect on words transmission over Keystone/Davinci SPI bus: +-+ +-+ +-+ +-+ +-+ +-+ +-+ +-+ SPI_CLK | | | | | | | | | | | | | | | | +--+ +-+ +-+ +-+ +-+ +---+ +-+ +-+ +- SPI_SOMI/SIMO+-+ +--- +--+ word1 +---+word2 +-+ +--- WDELAY - + + SPI_CS | | +++---+ || | +-+-++ +-+ | | || | | + + +| + + --- -- --- C2TDELAY T2CDELAYC2TDELAY Where: WDELAY - Delay in between transmissions C2TDELAY - Chip-select-active-to-transmit-start-delay T2CDELAY - Transmit-end-to-chip-select-inactive-delay Also, below is additional information about properties which are used in 5-pin mode (SPI_READY) to improve error detection [OMAP-L138/da830 - http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf]: - ti,davinci-spi-t2e-delay: t2e delay Transmit-data-finished-to-SPIx_ENA-pin-inactive-time-out. T2EDELAY is used in master mode only. It defines a time-out value as a multiple of SPI clock before the SPIx_ENA signal has to become inactive and after the CS becomes inactive. The SPI clock depends on which data format is selected. If the slave device is missing one or more clock edges, it is becoming desynchronized. Although the master has finished the data transfer the slave is still waiting for the missed clock pulses and the SPIx_ENA signal is not disabled. The T2EDELAY defines a time-out value that triggers the DESYNC flag, if the SPIx_ENA signal is not deactivated in time. The DESYNC flag is set to indicate that the slave device did not deassert its SPIx_ENA pin in time to acknowledge that it has received all the bits of the sent character. The DESYNC flag is also set if the SPI detects a deassertion of the SPIx_ENA pin even before the end of the transmission. - ti,davinci-spi-c2e-delay: c2e delay Chip-select-active-to-SPIx_ENA-signal-active-time-out. C2EDELAY is used only in master mode and it applies only if the addressed slave generates an SPIx_ENA signal as a hardware handshake response. C2EDELAY defines the maximum time between the SPI activates the chip select signal and the addressed slave has to respond by activating the SPIx_ENA signal. +- ti,davinci-spi-odd-parity : odd partity enabled + OR + ti,davinci-spi-even-parity : even parity enabled What do these mean? Supported by OMAP-L138/da830: SPIFMTn[23
Re: [PATCH 2/2] spi: davinci: support adding delay between transmission
Hi Mark, On 08/21/2014 09:20 PM, Mark Brown wrote: On Thu, Aug 21, 2014 at 06:25:06PM +0300, Grygorii Strashko wrote: +- ti,davinci-spi-wdelay : delay between transmissions. I don't understand why this is here - there is already standard support in the SPI core for client drivers specifying inter-transfer delays. If there is a need to provide platform configuration for this in addition to that then it should also be a standard property, there is nothing device specific about these. Sorry, may be I've missed smth, because I was not able to find such common property in SPI bindings document and code. Could you point me on, pls? In our case it's hardware delay applied between transmissions: Delay in between transmissions. Idle time that will be applied at the end of the current transmission if the bit WDEL is set in the current buffer. The delay to be applied is equal to: WDELAY × PSPI module clock + 2 × PSPI module clock PSPI module clock - Period of SPI module clock +- ti,davinci-spi-odd-parity : odd partity enabled + OR + ti,davinci-spi-even-parity : even parity enabled What do these mean? Supported by OMAP-L138/da830: SPIFMTn[23].PARPOL - Parity polarity: even or odd. PARPOL can be modified in privilege mode only. 0 An even parity flag is added at the end of the transmit data stream. 1 An odd parity flag is added at the end of the transmit data stream. SPIFMTn[22].PARENA - Parity enable. 0 - No parity generation/ verification is performed. 1 - A parity is transmitted at the end of each transmit data stream. At the end of a transfer the parity generator compares the received parity bit with the locally calculated parity flag. If the parity bits do not match the PARERR flag is set in the corresponding control field. The parity type (even or odd) can be selected via the PARPOL bit. +- ti,davinci-spi-io-type: io type (check platform_data/spi-davinci.h) The bindings should be independent of the kernel, the values need to be included here (and the defines moved to include/dt-bindings so they can be used when writing DTs). Allowed values here are: #define SPI_IO_TYPE_INTR0 #define SPI_IO_TYPE_POLL1 #define SPI_IO_TYPE_DMA 2 I'll update. +- ti,davinci-spi-disable-timer: disable CS timer (SPIFMTn) +- ti,davinci-spi-c2t-delay: c2t delay +- ti,davinci-spi-t2c-delay: t2c delay +- ti,davinci-spi-t2e-delay: t2e delay +- ti,davinci-spi-c2e-delay: c2e delay What are all these timers/delays - at least some reference to the datasheet please? Sorry, I'll update bindings with links on datasheet. Keystone 2: http://www.ti.com/lit/ug/sprugp2a/sprugp2a.pdf Davinci: dm644x - http://www.ti.com/lit/ug/sprue32a/sprue32a.pdf OMAP-L138/da830 - http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf Regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 1/2] spi: davinci: fix SPI_NO_CS functionality
The driver should not touch CS lines if SPI_NO_CS flag is set.
This patch fixes it as this functionality was broken accidentally
by
commit a88e34ea213e1b (spi: davinci: add support to configure gpio cs through
dt).
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/spi/spi-davinci.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 276a388..48f1d26 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -417,16 +417,16 @@ static int davinci_spi_setup(struct spi_device *spi)
flags, dev_name(spi-dev));
internal_cs = false;
}
- }
- if (retval) {
- dev_err(spi-dev, GPIO %d setup failed (%d)\n,
- spi-cs_gpio, retval);
- return retval;
- }
+ if (retval) {
+ dev_err(spi-dev, GPIO %d setup failed (%d)\n,
+ spi-cs_gpio, retval);
+ return retval;
+ }
- if (internal_cs)
- set_io_bits(dspi-base + SPIPC0, 1 spi-chip_select);
+ if (internal_cs)
+ set_io_bits(dspi-base + SPIPC0, 1 spi-chip_select);
+ }
if (spi-mode SPI_READY)
set_io_bits(dspi-base + SPIPC0, SPIPC0_SPIENA_MASK);
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 0/2] spi: davinci: fixes and updates
The first patch fixes the issue for the case when SPI_NO_CS flag is set which was introduced by previous commit a88e34ea213e1b (spi: davinci: add support to configure gpio cs through dt). The second patch completes migration of Davinci SPI driver to DT and adds ability to configure additional properties for SPI slave devices which were possible to configure only from platform code through struct davinci_spi_config Grygorii Strashko (2): spi: davinci: fix SPI_NO_CS functionality spi: davinci: support adding delay between transmission .../devicetree/bindings/spi/spi-davinci.txt| 18 drivers/spi/spi-davinci.c | 96 +--- 2 files changed, 99 insertions(+), 15 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 2/2] spi: davinci: support adding delay between transmission
This patch completes migration of Davinci SPI driver to DT
and adds ability to configure additional properties for
SPI slave devices which were possible to configure only
from platform code through struct davinci_spi_config before
this patch.
New optional SPI slave properties:
- ti,davinci-spi-wdelay : delay between transmissions.
- ti,davinci-spi-odd-parity : odd partity enabled
OR
ti,davinci-spi-even-parity : even parity enabled
- ti,davinci-spi-io-type: io type
- ti,davinci-spi-disable-timer: disable CS timer (SPIFMTn)
- ti,davinci-spi-c2t-delay: c2t delay
- ti,davinci-spi-t2c-delay: t2c delay
- ti,davinci-spi-t2e-delay: t2e delay
- ti,davinci-spi-c2e-delay: c2e delay
Signed-off-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/spi/spi-davinci.txt| 18 +
drivers/spi/spi-davinci.c | 80 ++--
2 files changed, 91 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt
b/Documentation/devicetree/bindings/spi/spi-davinci.txt
index f80887b..bfcd42d 100644
--- a/Documentation/devicetree/bindings/spi/spi-davinci.txt
+++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
@@ -24,6 +24,22 @@ Optional:
cs-gpios = 0, 0, 0, gpio1 30 0, gpio1 31 0;
where first three are internal CS and last two are GPIO CS.
+Optional properties for slave devices:
+SPI slave nodes can contain the following properties.
+Not all SPI Peripherals from Texas Instruments support this.
+Please check SPI peripheral documentation for a device before using these.
+
+- ti,davinci-spi-wdelay : delay between transmissions.
+- ti,davinci-spi-odd-parity : odd partity enabled
+ OR
+ ti,davinci-spi-even-parity : even parity enabled
+- ti,davinci-spi-io-type: io type (check platform_data/spi-davinci.h)
+- ti,davinci-spi-disable-timer: disable CS timer (SPIFMTn)
+- ti,davinci-spi-c2t-delay: c2t delay
+- ti,davinci-spi-t2c-delay: t2c delay
+- ti,davinci-spi-t2e-delay: t2e delay
+- ti,davinci-spi-c2e-delay: c2e delay
+
Example of a NOR flash slave device (n25q032) connected to DaVinci
SPI controller device over the SPI bus.
@@ -43,6 +59,8 @@ spi0:spi@20BF {
compatible = st,m25p32;
spi-max-frequency = 2500;
reg = 0;
+ ti,davinci-spi-c2t-delay = 8;
+ ti,davinci-spi-t2c-delay = 8;
partition@0 {
label = u-boot-spl;
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 48f1d26..0e276ad 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -65,6 +65,7 @@
/* SPIDAT1 (upper 16 bit defines) */
#define SPIDAT1_CSHOLD_MASKBIT(12)
+#define SPIDAT1_WDEL BIT(10)
/* SPIGCR1 */
#define SPIGCR1_CLKMOD_MASKBIT(1)
@@ -209,6 +210,7 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
{
struct davinci_spi *dspi;
struct davinci_spi_platform_data *pdata;
+ struct davinci_spi_config *spicfg = spi-controller_data;
u8 chip_sel = spi-chip_select;
u16 spidat1 = CS_DEFAULT;
bool gpio_chipsel = false;
@@ -223,6 +225,10 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
gpio = spi-cs_gpio;
}
+ /* program delay transfers if tx_delay is non zero */
+ if (spicfg-wdelay)
+ spidat1 |= SPIDAT1_WDEL;
+
/*
* Board specific chip select logic decides the polarity and cs
* line for the controller
@@ -237,9 +243,9 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
spidat1 |= SPIDAT1_CSHOLD_MASK;
spidat1 = ~(0x1 chip_sel);
}
-
- iowrite16(spidat1, dspi-base + SPIDAT1 + 2);
}
+
+ iowrite16(spidat1, dspi-base + SPIDAT1 + 2);
}
/**
@@ -285,7 +291,7 @@ static int davinci_spi_setup_transfer(struct spi_device
*spi,
int prescale;
dspi = spi_master_get_devdata(spi-master);
- spicfg = (struct davinci_spi_config *)spi-controller_data;
+ spicfg = spi-controller_data;
if (!spicfg)
spicfg = davinci_spi_default_cfg;
@@ -333,6 +339,14 @@ static int davinci_spi_setup_transfer(struct spi_device
*spi,
spifmt |= SPIFMT_PHASE_MASK;
/*
+ * Assume wdelay is used only on SPI peripherals that has this field
+ * in SPIFMTn register and when it's configured from board file or DT.
+ */
+ if (spicfg-wdelay)
+ spifmt |= ((spicfg-wdelay SPIFMT_WDELAY_SHIFT)
+SPIFMT_WDELAY_MASK);
+
+ /*
* Version 1 hardware supports two basic SPI modes:
* - Standard SPI mode uses 4 pins, with chipselect
* - 3 pin SPI is a 4 pin variant without CS (SPI_NO_CS)
@@ -349,9 +363,6
[PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt
From: Murali Karicheri m-kariche...@ti.com
Currently driver supports only configuration of GPIO CS through
platform data. This patch enhances the driver to configure GPIO
CS through DT. Also update the DT binding documentation to
reflect the availability of cs-gpios.
Signed-off-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/spi/spi-davinci.txt|9 ++-
drivers/spi/spi-davinci.c | 64 +---
2 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt
b/Documentation/devicetree/bindings/spi/spi-davinci.txt
index 6d0ac8d..f80887b 100644
--- a/Documentation/devicetree/bindings/spi/spi-davinci.txt
+++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
@@ -8,7 +8,8 @@ Required properties:
- ti,dm6441-spi for SPI used similar to that on DM644x SoC family
- ti,da830-spi for SPI used similar to that on DA8xx SoC family
- reg: Offset and length of SPI controller register space
-- num-cs: Number of chip selects
+- num-cs: Number of chip selects. This includes internal as well as
+ GPIO chip selects.
- ti,davinci-spi-intr-line: interrupt line used to connect the SPI
IP to the interrupt controller within the SoC. Possible values
are 0 and 1. Manual says one of the two possible interrupt
@@ -17,6 +18,12 @@ Required properties:
- interrupts: interrupt number mapped to CPU.
- clocks: spi clk phandle
+Optional:
+- cs-gpios: gpio chip selects
+ For example to have 3 internal CS and 2 GPIO CS, user could define
+ cs-gpios = 0, 0, 0, gpio1 30 0, gpio1 31 0;
+ where first three are internal CS and last two are GPIO CS.
+
Example of a NOR flash slave device (n25q032) connected to DaVinci
SPI controller device over the SPI bus.
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 2477af4..ac4414e0 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -30,6 +30,7 @@
#include linux/edma.h
#include linux/of.h
#include linux/of_device.h
+#include linux/of_gpio.h
#include linux/spi/spi.h
#include linux/spi/spi_bitbang.h
#include linux/slab.h
@@ -207,17 +208,28 @@ static inline void clear_io_bits(void __iomem *addr, u32
bits)
static void davinci_spi_chipselect(struct spi_device *spi, int value)
{
struct davinci_spi *dspi;
+ struct device_node *np = spi-dev.of_node;
struct davinci_spi_platform_data *pdata;
+ struct spi_master *master = spi-master;
u8 chip_sel = spi-chip_select;
u16 spidat1 = CS_DEFAULT;
bool gpio_chipsel = false;
+ int gpio;
dspi = spi_master_get_devdata(spi-master);
pdata = dspi-pdata;
- if (pdata-chip_sel chip_sel pdata-num_chipselect
- pdata-chip_sel[chip_sel] != SPI_INTERN_CS)
+ if (np master-cs_gpios != NULL spi-cs_gpio = 0) {
+ /* SPI core parse and update master-cs_gpio */
gpio_chipsel = true;
+ gpio = spi-cs_gpio;
+ } else if (pdata-chip_sel
+ chip_sel pdata-num_chipselect
+ pdata-chip_sel[chip_sel] != SPI_INTERN_CS) {
+ /* platform data defines chip_sel */
+ gpio_chipsel = true;
+ gpio = pdata-chip_sel[chip_sel];
+ }
/*
* Board specific chip select logic decides the polarity and cs
@@ -225,9 +237,9 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
*/
if (gpio_chipsel) {
if (value == BITBANG_CS_ACTIVE)
- gpio_set_value(pdata-chip_sel[chip_sel], 0);
+ gpio_set_value(gpio, 0);
else
- gpio_set_value(pdata-chip_sel[chip_sel], 1);
+ gpio_set_value(gpio, 1);
} else {
if (value == BITBANG_CS_ACTIVE) {
spidat1 |= SPIDAT1_CSHOLD_MASK;
@@ -390,17 +402,41 @@ static int davinci_spi_setup(struct spi_device *spi)
int retval = 0;
struct davinci_spi *dspi;
struct davinci_spi_platform_data *pdata;
+ struct spi_master *master = spi-master;
+ struct device_node *np = spi-dev.of_node;
+ bool internal_cs = true;
dspi = spi_master_get_devdata(spi-master);
pdata = dspi-pdata;
if (!(spi-mode SPI_NO_CS)) {
- if ((pdata-chip_sel == NULL) ||
- (pdata-chip_sel[spi-chip_select] == SPI_INTERN_CS))
- set_io_bits(dspi-base + SPIPC0, 1 spi-chip_select);
-
+ if (np (master-cs_gpios != NULL) (spi-cs_gpio = 0)) {
+ unsigned long flags;
+
+ flags = GPIOF_DIR_OUT;
+ if (spi-mode SPI_CS_HIGH)
+ flags |= GPIOF_INIT_LOW
[PATCH v2 0/2] spi: davinci: add support for gpio cs through dt
This small series enables GPIO chip select feature for Davinci SPI when DT-boot mode is used. This is actual for Keystone 2 which supports DT-boot mode only. Changes in v2: - gpio_request_one() is used - as suggested by Mark Brown, new patch has been added which updates Davinci SPI driver to use cs_gpio field of SPI device structure (spi_device) for both DT and non-DT cases. v1: http://www.spinics.net/lists/linux-spi/msg01446.html Grygorii Strashko (1): spi: davinci: use spi_device.cs_gpio to store gpio cs per spi device Murali Karicheri (1): spi: davinci: add support to configure gpio cs through dt .../devicetree/bindings/spi/spi-davinci.txt|9 ++- drivers/spi/spi-davinci.c | 60 ++-- 2 files changed, 52 insertions(+), 17 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 2/2] spi: davinci: use spi_device.cs_gpio to store gpio cs per spi device
Rework Davinci SPI driver to store GPIO CS number in cs_gpio field
of SPI device structure (spi_device) for both DT and non-DT cases.
This will make Davinci SPI driver code simpler and allows to reuse
more SPI core functionality.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/spi/spi-davinci.c | 54 ++---
1 file changed, 17 insertions(+), 37 deletions(-)
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index ac4414e0..276a388 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -208,9 +208,7 @@ static inline void clear_io_bits(void __iomem *addr, u32
bits)
static void davinci_spi_chipselect(struct spi_device *spi, int value)
{
struct davinci_spi *dspi;
- struct device_node *np = spi-dev.of_node;
struct davinci_spi_platform_data *pdata;
- struct spi_master *master = spi-master;
u8 chip_sel = spi-chip_select;
u16 spidat1 = CS_DEFAULT;
bool gpio_chipsel = false;
@@ -219,16 +217,10 @@ static void davinci_spi_chipselect(struct spi_device
*spi, int value)
dspi = spi_master_get_devdata(spi-master);
pdata = dspi-pdata;
- if (np master-cs_gpios != NULL spi-cs_gpio = 0) {
+ if (spi-cs_gpio = 0) {
/* SPI core parse and update master-cs_gpio */
gpio_chipsel = true;
gpio = spi-cs_gpio;
- } else if (pdata-chip_sel
- chip_sel pdata-num_chipselect
- pdata-chip_sel[chip_sel] != SPI_INTERN_CS) {
- /* platform data defines chip_sel */
- gpio_chipsel = true;
- gpio = pdata-chip_sel[chip_sel];
}
/*
@@ -237,9 +229,9 @@ static void davinci_spi_chipselect(struct spi_device *spi,
int value)
*/
if (gpio_chipsel) {
if (value == BITBANG_CS_ACTIVE)
- gpio_set_value(gpio, 0);
+ gpio_set_value(gpio, spi-mode SPI_CS_HIGH);
else
- gpio_set_value(gpio, 1);
+ gpio_set_value(gpio, !(spi-mode SPI_CS_HIGH));
} else {
if (value == BITBANG_CS_ACTIVE) {
spidat1 |= SPIDAT1_CSHOLD_MASK;
@@ -405,35 +397,34 @@ static int davinci_spi_setup(struct spi_device *spi)
struct spi_master *master = spi-master;
struct device_node *np = spi-dev.of_node;
bool internal_cs = true;
+ unsigned long flags = GPIOF_DIR_OUT;
dspi = spi_master_get_devdata(spi-master);
pdata = dspi-pdata;
+ flags |= (spi-mode SPI_CS_HIGH) ? GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
+
if (!(spi-mode SPI_NO_CS)) {
if (np (master-cs_gpios != NULL) (spi-cs_gpio = 0)) {
- unsigned long flags;
-
- flags = GPIOF_DIR_OUT;
- if (spi-mode SPI_CS_HIGH)
- flags |= GPIOF_INIT_LOW;
- else
- flags |= GPIOF_INIT_HIGH;
retval = gpio_request_one(spi-cs_gpio,
flags, dev_name(spi-dev));
- if (retval) {
- dev_err(spi-dev,
- GPIO %d request failed (%d)\n,
- spi-cs_gpio, retval);
- return retval;
- }
internal_cs = false;
} else if (pdata-chip_sel
spi-chip_select pdata-num_chipselect
pdata-chip_sel[spi-chip_select] != SPI_INTERN_CS) {
+ spi-cs_gpio = pdata-chip_sel[spi-chip_select];
+ retval = gpio_request_one(spi-cs_gpio,
+ flags, dev_name(spi-dev));
internal_cs = false;
}
}
+ if (retval) {
+ dev_err(spi-dev, GPIO %d setup failed (%d)\n,
+ spi-cs_gpio, retval);
+ return retval;
+ }
+
if (internal_cs)
set_io_bits(dspi-base + SPIPC0, 1 spi-chip_select);
@@ -450,10 +441,7 @@ static int davinci_spi_setup(struct spi_device *spi)
static void davinci_spi_cleanup(struct spi_device *spi)
{
- struct spi_master *master = spi-master;
- struct device_node *np = spi-dev.of_node;
-
- if (np (master-cs_gpios != NULL) (spi-cs_gpio = 0))
+ if (spi-cs_gpio = 0)
gpio_free(spi-cs_gpio);
}
@@ -895,7 +883,7 @@ static int davinci_spi_probe(struct platform_device *pdev)
struct resource *r;
resource_size_t dma_rx_chan = SPI_NO_RESOURCE;
resource_size_t dma_tx_chan = SPI_NO_RESOURCE;
- int i = 0, ret = 0;
+ int ret = 0;
u32 spipc0
Re: [PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt
On 08/01/2014 08:26 PM, Mark Brown wrote: On Fri, Aug 01, 2014 at 07:40:32PM +0300, Grygorii Strashko wrote: From: Murali Karicheri m-kariche...@ti.com Currently driver supports only configuration of GPIO CS through platform data. This patch enhances the driver to configure GPIO CS through DT. Also update the DT binding documentation to reflect the availability of cs-gpios. Please submit an incremental patch with whatever the changes were from the version of this that I applied already. This series is based on top of: [PATCH 1/2] spi: davinci: fix to support more than 2 chip selects http://www.spinics.net/lists/linux-spi/msg01447.html Sorry, I've forgot to mention this in cover letter. Is that what you mean? regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt
On 08/01/2014 08:35 PM, Mark Brown wrote:
On Fri, Aug 01, 2014 at 07:40:32PM +0300, Grygorii Strashko wrote:
From: Murali Karicheri m-kariche...@ti.com
Currently driver supports only configuration of GPIO CS through
platform data. This patch enhances the driver to configure GPIO
CS through DT. Also update the DT binding documentation to
reflect the availability of cs-gpios.
Ah, sorry - this is the unapplied patch from before. It would have been
much better to submit this as the second patch after your rework patch
since this...
I can do this, but It will wait for a week :(
-if (pdata-chip_sel chip_sel pdata-num_chipselect
-pdata-chip_sel[chip_sel] != SPI_INTERN_CS)
+if (np master-cs_gpios != NULL spi-cs_gpio = 0) {
+/* SPI core parse and update master-cs_gpio */
gpio_chipsel = true;
+gpio = spi-cs_gpio;
+} else if (pdata-chip_sel
+ chip_sel pdata-num_chipselect
+ pdata-chip_sel[chip_sel] != SPI_INTERN_CS) {
+/* platform data defines chip_sel */
+gpio_chipsel = true;
+gpio = pdata-chip_sel[chip_sel];
+}
...still looks excessively confusing - it is way more logic than I'd
expect to see on every chip select.
Yep. Finally, patch [PATCH v2 2/2] spi: davinci: use spi_device.cs_gpio to
store gpio cs per spi device
simplifies logic above.
Regards,
-grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: i2c-davinci.c: CPU FREQ causes lock up due to xfr_complete
On 07/30/2014 09:18 AM, Sekhar Nori wrote:
On Tuesday 29 July 2014 11:00 PM, Grygorii Strashko wrote:
Hi Jon,
On 07/29/2014 06:53 PM, Jon Cormier wrote:
A slimmer patch suggested by Grygorii Strashko grygorii.stras...@ti.com
Ok. The problem seems to be deeper than at first look.
You have sequence (in Mainline kernel):
cpufreq:
|- notify CPUFREQ_PRECHANGE
|- i2c-davinci will lock put I2C in reset
|- cpufreq_driver-target_index
|- davinci_target()
|- pdata-set_voltage() - It will try to use I2C to set new voltage,
while I2C is in reset or locked! Bug!
|- notify CPUFREQ_POSTCHANGE
|- i2c-davinci will re-enable I2C and adjust I2C clock
Good find. I really wonder how this escaped so far. I can swear cpufreq
transitions were tested multiple times. From the description it looks
like this should hit every single time there is a voltage adjustment.
On DA850 which is the only DaVinci implementing cpufreq, I2C0 input
frequency will remain constant across cpufreq transitions since it
derives from PLL0 AUXCLK which is used pre-multipler/divider. It remains
fixed.
The code seems to have been added for I2C1 which does have a fixed ratio
with cpu clock.
PMIC should usually be put on I2C0 to help prevent these kind of issues.
I see few possible ways to solve it:
1) use CLK notifier instead of CPUfreq notifiers
This will require common clock framework, right? We dont have that on
mach-davinci.
:( I've forgotten about that.
2) do smth similar to 61c7cff8 i2c: S3C24XX I2C frequency scaling support
+ 9bcd04bf i2c: s3c2410: grab adapter lock while changing i2c clock
This looks promising. Although I wonder if delta_f will always remain
zero in s3c24xx_i2c_cpufreq_transition() when the CPUFREQ_PRECHANGE call
is made - because clock tree is not updated yet?
That's funny - seems PRE/POST notifiers are called twice for s3c24xx :)
First one from cpufreq core.
Second time from s3c_cpufreq_target() and, looks like, clock
freq will be updated at that time.
3) update I2C clock in CPUFREQ_POSTCHANGE - may be unsafe
Well, even now the I2C clock is only getting updated in POSTCHANGE,
right? Also, resetting I2C in PRECHANGE seems excessive. It is only
required when changing the prescalar. So you can do:
} else if (val == CPUFREQ_POSTCHANGE) {
davinci_i2c_reset_ctrl(dev, 0);
i2c_davinci_calc_clk_dividers(dev);
davinci_i2c_reset_ctrl(dev, 1);
}
So this along with the i2c_lock_adapter() a la
s3c24xx_i2c_cpufreq_transition() should be the right fix, I guess.
Regards,
-grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: i2c-davinci.c: CPU FREQ causes lock up due to xfr_complete
Hi Jon,
On 07/29/2014 06:53 PM, Jon Cormier wrote:
A slimmer patch suggested by Grygorii Strashko grygorii.stras...@ti.com
Ok. The problem seems to be deeper than at first look.
You have sequence (in Mainline kernel):
cpufreq:
|- notify CPUFREQ_PRECHANGE
|- i2c-davinci will lock put I2C in reset
|- cpufreq_driver-target_index
|- davinci_target()
|- pdata-set_voltage() - It will try to use I2C to set new voltage,
while I2C is in reset or locked! Bug!
|- notify CPUFREQ_POSTCHANGE
|- i2c-davinci will re-enable I2C and adjust I2C clock
I see few possible ways to solve it:
1) use CLK notifier instead of CPUfreq notifiers
2) do smth similar to 61c7cff8 i2c: S3C24XX I2C frequency scaling support
+ 9bcd04bf i2c: s3c2410: grab adapter lock while changing i2c clock
3) update I2C clock in CPUFREQ_POSTCHANGE - may be unsafe
Regards,
-grygorii
Author: Cormier, Jonathan jcorm...@criticallink.com
Date: Tue Jul 29 11:50:04 2014 -0400
i2c: davinci: Change xfr_complete completion to use i2c_lock_adapter
There are several problems with the use of a completion for this task:
1. If no I2C transfer has occurred, a cpufreq transition will block
forever.
2. Once an I2C transfer has occurred the cpufreq transition will
never block since the completion is never reinitialized.
3. Even if you do reinitialize the completion for every I2C
transfer, (1) is not solved and there is still a race condition where
the cpufreq transition could start just before an I2C transfer starts
and the I2C transfer occurs during the cpufreq transition.
Author: Grygorii Strashko grygorii.stras...@ti.com
Author: Cormier, Jonathan jcorm...@criticallink.com
Signed-off-by: Cormier, Jonathan jcorm...@criticallink.com
diff --git a/drivers/i2c/busses/i2c-davinci.c
b/drivers/i2c/busses/i2c-davinci.c
index a76d85f..f8e7b7f 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -111,7 +111,6 @@ struct davinci_i2c_dev {
u8terminate;
struct i2c_adapteradapter;
#ifdef CONFIG_CPU_FREQ
-struct completionxfr_complete;
struct notifier_blockfreq_transition;
#endif
};
@@ -452,10 +451,6 @@ i2c_davinci_xfer(struct i2c_adapter *adap, struct
i2c_msg msgs[], int num)
return ret;
}
-#ifdef CONFIG_CPU_FREQ
-complete(dev-xfr_complete);
-#endif
-
return num;
}
@@ -596,11 +591,12 @@ static int i2c_davinci_cpufreq_transition(struct
notifier_block *nb,
dev = container_of(nb, struct davinci_i2c_dev, freq_transition);
if (val == CPUFREQ_PRECHANGE) {
-wait_for_completion(dev-xfr_complete);
+i2c_lock_adapter(dev-adapter);
davinci_i2c_reset_ctrl(dev, 0);
} else if (val == CPUFREQ_POSTCHANGE) {
i2c_davinci_calc_clk_dividers(dev);
davinci_i2c_reset_ctrl(dev, 1);
+i2c_unlock_adapter(dev-adapter);
}
return 0;
@@ -669,9 +665,7 @@ static int davinci_i2c_probe(struct platform_device *pdev)
}
init_completion(dev-cmd_complete);
-#ifdef CONFIG_CPU_FREQ
-init_completion(dev-xfr_complete);
-#endif
+
dev-dev = get_device(pdev-dev);
dev-irq = irq-start;
platform_set_drvdata(pdev, dev);
On Tue, Jul 29, 2014 at 11:36 AM, Jon Cormier jcorm...@criticallink.com
wrote:
Okay here is my attempt.
Author: Cormier, Jonathan jcorm...@criticallink.com
Date: Tue Jul 29 11:26:22 2014 -0400
i2c: davinci: Change xfr_complete completion to a mutex
There are several problems with the use of a completion for this task:
1. If no I2C transfer has occurred, a cpufreq transition will block
forever.
2. Once an I2C transfer has occurred the cpufreq transition will
never block since the completion is never reinitialized.
3. Even if you do reinitialize the completion for every I2C
transfer, (1) is not solved and there is still a race condition where
the cpufreq transition could start just before an I2C transfer starts
and the I2C transfer occurs during the cpufreq transition.
Signed-off-by: Cormier, Jonathan jcorm...@criticallink.com
diff --git a/drivers/i2c/busses/i2c-davinci.c
b/drivers/i2c/busses/i2c-davinci.c
index a76d85f..9eac1c1 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -111,7 +111,7 @@ struct davinci_i2c_dev {
u8terminate;
struct i2c_adapteradapter;
#ifdef CONFIG_CPU_FREQ
-struct completionxfr_complete;
+struct mutexxfr_lock;
struct notifier_blockfreq_transition;
#endif
};
@@ -438,10 +438,14 @@ i2c_davinci_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[], int num)
dev_dbg(dev-dev, %s: msgs: %d\n, __func__, num);
+#ifdef CONFIG_CPU_FREQ
+mutex_lock(dev-xfr_lock);
+#endif
+
ret = i2c_davinci_wait_bus_not_busy(dev, 1
[PATCH v2 0/2] net: davinci_mdio: reuse for keystone2 arch
The similar MDIO HW blocks is used by keystone 2 SoCs as in Davinci SoCs: - one in Gigabit Ethernet (GbE) Switch Subsystem See http://www.ti.com/lit/ug/sprugv9d/sprugv9d.pdf - one in 10 Gigabit Ethernet Subsystem See http://www.ti.com/lit/ug/spruhj5/spruhj5.pdf Hence, reuse Davinci MDIO driver for Keystone 2 and enable TI networking for Keystone 2 devices. Also, as part of this series, enable PHY's creation from DT, because Keystone 2 supports DT boot mode only. Changes in v2: - review comments applied. Keystone 2 compatibility string changed to ti,keystone_mdio. Grygorii Strashko (2): net: davinci_mdio: reuse for keystone2 arch net: davinci_mdio: allow to create phys from dt .../devicetree/bindings/net/davinci-mdio.txt |8 drivers/net/ethernet/ti/Kconfig|4 ++-- drivers/net/ethernet/ti/davinci_mdio.c | 21 ++-- 3 files changed, 25 insertions(+), 8 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 1/2] net: davinci_mdio: reuse for keystone2 arch
The similar MDIO HW blocks is used by keystone 2 SoCs as
in Davinci SoCs:
- one in Gigabit Ethernet (GbE) Switch Subsystem
See http://www.ti.com/lit/ug/sprugv9d/sprugv9d.pdf
- one in 10 Gigabit Ethernet Subsystem
See http://www.ti.com/lit/ug/spruhj5/spruhj5.pdf
Hence, reuse Davinci MDIO driver for Keystone 2 and
enable TI networking for Keystone 2 devices
Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Mugunthan V N mugunthan...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/net/davinci-mdio.txt |8
drivers/net/ethernet/ti/Kconfig|4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt
b/Documentation/devicetree/bindings/net/davinci-mdio.txt
index 72efaaf..0369e25 100644
--- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
+++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
@@ -1,8 +1,8 @@
-TI SoC Davinci MDIO Controller Device Tree Bindings
+TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
---
Required properties:
-- compatible : Should be ti,davinci_mdio
+- compatible : Should be ti,davinci_mdio or ti,keystone_mdio
- reg : physical base address and size of the davinci mdio
registers map
- bus_freq : Mdio Bus frequency
@@ -19,7 +19,7 @@ file.
Examples:
mdio: davinci_mdio@4A101000 {
- compatible = ti,cpsw;
+ compatible = ti,davinci_mdio;
reg = 0x4A101000 0x1000;
bus_freq = 100;
};
@@ -27,7 +27,7 @@ Examples:
(or)
mdio: davinci_mdio@4A101000 {
- compatible = ti,cpsw;
+ compatible = ti,davinci_mdio;
ti,hwmods = davinci_mdio;
bus_freq = 100;
};
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 53150c2..1769700 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -5,7 +5,7 @@
config NET_VENDOR_TI
bool Texas Instruments (TI) devices
default y
- depends on PCI || EISA || AR7 || (ARM (ARCH_DAVINCI || ARCH_OMAP3 ||
SOC_AM33XX))
+ depends on PCI || EISA || AR7 || (ARM (ARCH_DAVINCI || ARCH_OMAP3 ||
SOC_AM33XX || ARCH_KEYSTONE))
---help---
If you have a network (Ethernet) card belonging to this class, say Y
and read the Ethernet-HOWTO, available from
@@ -32,7 +32,7 @@ config TI_DAVINCI_EMAC
config TI_DAVINCI_MDIO
tristate TI DaVinci MDIO Support
- depends on ARM ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
+ depends on ARM ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX ||
ARCH_KEYSTONE )
select PHYLIB
---help---
This driver supports TI's DaVinci MDIO module.
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 2/2] net: davinci_mdio: allow to create phys from dt
This patch allows to create PHYs from DT in case
if they are explicitly defined. The of_mdiobus_register() is
used for such purposes.
For backward compatibility, call of_mdiobus_register() only in case
if at least one PHY's child is defined in DT, otherwise rollback to
mdiobus_register().
Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Mugunthan V N mugunthan...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Hi Santosh,
I haven't changed style for long-comments, because Networking subsystem
requires first line of comment not to be empty:
WARNING:NETWORKING_BLOCK_COMMENT_STYLE: networking block comments don't use an
empty /* line, use /* Comment...
drivers/net/ethernet/ti/davinci_mdio.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 735dc53..2791f6f 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -38,6 +38,7 @@
#include linux/davinci_emac.h
#include linux/of.h
#include linux/of_device.h
+#include linux/of_mdio.h
#include linux/pinctrl/consumer.h
/*
@@ -95,6 +96,10 @@ struct davinci_mdio_data {
struct mii_bus *bus;
boolsuspended;
unsigned long access_time; /* jiffies */
+ /* Indicates that driver shouldn't modify phy_mask in case
+* if MDIO bus is registered from DT.
+*/
+ boolskip_scan;
};
static void __davinci_mdio_reset(struct davinci_mdio_data *data)
@@ -144,6 +149,9 @@ static int davinci_mdio_reset(struct mii_bus *bus)
dev_info(data-dev, davinci mdio revision %d.%d\n,
(ver 8) 0xff, ver 0xff);
+ if (data-skip_scan)
+ return 0;
+
/* get phy mask from the alive register */
phy_mask = __raw_readl(data-regs-alive);
if (phy_mask) {
@@ -369,8 +377,17 @@ static int davinci_mdio_probe(struct platform_device *pdev)
goto bail_out;
}
- /* register the mii bus */
- ret = mdiobus_register(data-bus);
+ /* register the mii bus
+* Create PHYs from DT only in case if PHY child nodes are explicitly
+* defined to support backward compatibility with DTs which assume that
+* Davinci MDIO will always scan the bus for PHYs detection.
+*/
+ if (dev-of_node of_get_child_count(dev-of_node)) {
+ data-skip_scan = true;
+ ret = of_mdiobus_register(data-bus, dev-of_node);
+ } else {
+ ret = mdiobus_register(data-bus);
+ }
if (ret)
goto bail_out;
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/2] net: davinci_mdio: reuse for keystone2 arch
On 07/10/2014 10:39 PM, David Miller wrote: From: Grygorii Strashko grygorii.stras...@ti.com Date: Thu, 10 Jul 2014 15:58:31 +0300 Hi David, On 07/10/2014 02:52 AM, David Miller wrote: From: Grygorii Strashko grygorii.stras...@ti.com Date: Wed, 9 Jul 2014 16:10:50 +0300 Required properties: -- compatible : Should be ti,davinci_mdio +- compatible : Should be ti,davinci_mdio or ti,keystone-mdio Why the inconsistency in naming schemes? I don't see any reason to be different wrt. _ vs. - in the name string. Hm. Looks like the common way is to use -, but I can rename it if you insist. I'm just saying, is there a strong reason to be inconsistent? I've followed the same format as for all latest compatibility strings in Kernel. Also I've checked ePAPR and dash is used for all examples there. ti,davinci_mdio was added 2 years ago, so possibly no strict convention or review were done then. Now, I can't change ti,davinci_mdio - ti,davinci-mdio to be consistent with Kernel due to compatibility issues. May be DT Gurus can say more? Regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/2] net: davinci_mdio: reuse for keystone2 arch
Hi David, On 07/10/2014 02:52 AM, David Miller wrote: From: Grygorii Strashko grygorii.stras...@ti.com Date: Wed, 9 Jul 2014 16:10:50 +0300 Required properties: -- compatible: Should be ti,davinci_mdio +- compatible: Should be ti,davinci_mdio or ti,keystone-mdio Why the inconsistency in naming schemes? I don't see any reason to be different wrt. _ vs. - in the name string. Hm. Looks like the common way is to use -, but I can rename it if you insist. Regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 0/2] net: davinci_mdio: reuse for keystone2 arch
The similar MDIO HW blocks is used by keystone 2 SoCs as in Davinci SoCs: - one in Gigabit Ethernet (GbE) Switch Subsystem See http://www.ti.com/lit/ug/sprugv9d/sprugv9d.pdf - one in 10 Gigabit Ethernet Subsystem See http://www.ti.com/lit/ug/spruhj5/spruhj5.pdf Hence, reuse Davinci MDIO driver for Keystone 2 and enable TI networking for Keystone 2 devices. Also, as part of this series, enable PHY's creation from DT, because Keystone 2 supports DT boot mode only. Grygorii Strashko (2): net: davinci_mdio: reuse for keystone2 arch net: davinci_mdio: allow to create phys from dt .../devicetree/bindings/net/davinci-mdio.txt |8 drivers/net/ethernet/ti/Kconfig|4 ++-- drivers/net/ethernet/ti/davinci_mdio.c | 18 -- 3 files changed, 22 insertions(+), 8 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 1/2] net: davinci_mdio: reuse for keystone2 arch
The similar MDIO HW blocks is used by keystone 2 SoCs as
in Davinci SoCs:
- one in Gigabit Ethernet (GbE) Switch Subsystem
See http://www.ti.com/lit/ug/sprugv9d/sprugv9d.pdf
- one in 10 Gigabit Ethernet Subsystem
See http://www.ti.com/lit/ug/spruhj5/spruhj5.pdf
Hence, reuse Davinci MDIO driver for Keystone 2 and
enable TI networking for Keystone 2 devices
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/net/davinci-mdio.txt |8
drivers/net/ethernet/ti/Kconfig|4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt
b/Documentation/devicetree/bindings/net/davinci-mdio.txt
index 72efaaf..d2e68e7 100644
--- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
+++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
@@ -1,8 +1,8 @@
-TI SoC Davinci MDIO Controller Device Tree Bindings
+TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
---
Required properties:
-- compatible : Should be ti,davinci_mdio
+- compatible : Should be ti,davinci_mdio or ti,keystone-mdio
- reg : physical base address and size of the davinci mdio
registers map
- bus_freq : Mdio Bus frequency
@@ -19,7 +19,7 @@ file.
Examples:
mdio: davinci_mdio@4A101000 {
- compatible = ti,cpsw;
+ compatible = ti,davinci_mdio;
reg = 0x4A101000 0x1000;
bus_freq = 100;
};
@@ -27,7 +27,7 @@ Examples:
(or)
mdio: davinci_mdio@4A101000 {
- compatible = ti,cpsw;
+ compatible = ti,davinci_mdio;
ti,hwmods = davinci_mdio;
bus_freq = 100;
};
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 53150c2..1769700 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -5,7 +5,7 @@
config NET_VENDOR_TI
bool Texas Instruments (TI) devices
default y
- depends on PCI || EISA || AR7 || (ARM (ARCH_DAVINCI || ARCH_OMAP3 ||
SOC_AM33XX))
+ depends on PCI || EISA || AR7 || (ARM (ARCH_DAVINCI || ARCH_OMAP3 ||
SOC_AM33XX || ARCH_KEYSTONE))
---help---
If you have a network (Ethernet) card belonging to this class, say Y
and read the Ethernet-HOWTO, available from
@@ -32,7 +32,7 @@ config TI_DAVINCI_EMAC
config TI_DAVINCI_MDIO
tristate TI DaVinci MDIO Support
- depends on ARM ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
+ depends on ARM ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX ||
ARCH_KEYSTONE )
select PHYLIB
---help---
This driver supports TI's DaVinci MDIO module.
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 2/2] net: davinci_mdio: allow to create phys from dt
This patch allows to create PHYs from DT in case
if they are explicitly defined. The of_mdiobus_register() is
used for such purposes.
For backward compatibility, call of_mdiobus_register() only in case
if at least one PHY's child is defined in DT, otherwise rollback to
mdiobus_register().
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/net/ethernet/ti/davinci_mdio.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 735dc53..22d3dab 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -38,6 +38,7 @@
#include linux/davinci_emac.h
#include linux/of.h
#include linux/of_device.h
+#include linux/of_mdio.h
#include linux/pinctrl/consumer.h
/*
@@ -95,6 +96,7 @@ struct davinci_mdio_data {
struct mii_bus *bus;
boolsuspended;
unsigned long access_time; /* jiffies */
+ boolskip_scan;
};
static void __davinci_mdio_reset(struct davinci_mdio_data *data)
@@ -144,6 +146,9 @@ static int davinci_mdio_reset(struct mii_bus *bus)
dev_info(data-dev, davinci mdio revision %d.%d\n,
(ver 8) 0xff, ver 0xff);
+ if (data-skip_scan)
+ return 0;
+
/* get phy mask from the alive register */
phy_mask = __raw_readl(data-regs-alive);
if (phy_mask) {
@@ -369,8 +374,17 @@ static int davinci_mdio_probe(struct platform_device *pdev)
goto bail_out;
}
- /* register the mii bus */
- ret = mdiobus_register(data-bus);
+ /* register the mii bus
+* Create PHYs from DT only in case if PHY child nodes are explicitly
+* defined to support backward compatibility with DTs which assume that
+* Davinci MDIO will always scan the bus for PHYs detection.
+*/
+ if (dev-of_node of_get_child_count(dev-of_node)) {
+ data-skip_scan = true;
+ ret = of_mdiobus_register(data-bus, dev-of_node);
+ } else {
+ ret = mdiobus_register(data-bus);
+ }
if (ret)
goto bail_out;
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v3 2/4] net: davinci_mdio: use devm_* api
Use devm_* API for memory allocation and to get device's clock
to simplify driver's code.
Cc: Florian Fainelli f.faine...@gmail.com
Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
Acked-and-tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/net/ethernet/ti/davinci_mdio.c | 24 +---
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 0cca9de..4b202a2 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -321,15 +321,14 @@ static int davinci_mdio_probe(struct platform_device
*pdev)
struct phy_device *phy;
int ret, addr;
- data = kzalloc(sizeof(*data), GFP_KERNEL);
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
- data-bus = mdiobus_alloc();
+ data-bus = devm_mdiobus_alloc(dev);
if (!data-bus) {
dev_err(dev, failed to alloc mii bus\n);
- ret = -ENOMEM;
- goto bail_out;
+ return -ENOMEM;
}
if (dev-of_node) {
@@ -354,7 +353,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
pm_runtime_enable(pdev-dev);
pm_runtime_get_sync(pdev-dev);
- data-clk = clk_get(pdev-dev, fck);
+ data-clk = devm_clk_get(dev, fck);
if (IS_ERR(data-clk)) {
dev_err(dev, failed to get device clock\n);
ret = PTR_ERR(data-clk);
@@ -406,16 +405,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
return 0;
bail_out:
- if (data-bus)
- mdiobus_free(data-bus);
-
- if (data-clk)
- clk_put(data-clk);
pm_runtime_put_sync(pdev-dev);
pm_runtime_disable(pdev-dev);
- kfree(data);
-
return ret;
}
@@ -423,18 +415,12 @@ static int davinci_mdio_remove(struct platform_device
*pdev)
{
struct davinci_mdio_data *data = platform_get_drvdata(pdev);
- if (data-bus) {
+ if (data-bus)
mdiobus_unregister(data-bus);
- mdiobus_free(data-bus);
- }
- if (data-clk)
- clk_put(data-clk);
pm_runtime_put_sync(pdev-dev);
pm_runtime_disable(pdev-dev);
- kfree(data);
-
return 0;
}
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v3 0/4] introduce devm_mdiobus_alloc/free and clean up davinci mdio
Introduce a resource managed devm_mdiobus_alloc[_size]()/devm_mdiobus_free() to automatically clean up MDIO bus alocations made by MDIO drivers, thus leading to simplified MDIO drivers code. Clean up Davinci MDIO driver and use new devm API. Changes in v3: - added devm_mdiobus_alloc_size() and devm_mdiobus_alloc() converted to be just a simple wrapper now. Changes in v2: - minor comments taken into account - additional patches added for cleaning up Davinci MDIO driver Cc: Florian Fainelli f.faine...@gmail.com Grygorii Strashko (4): mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free net: davinci_mdio: use devm_* api net: davinci_mdio: drop pinctrl_pm_select_default_state from probe net: davinci_mdio: simplify IO memory mapping Documentation/driver-model/devres.txt |5 +++ drivers/net/ethernet/ti/davinci_mdio.c | 48 --- drivers/net/phy/mdio_bus.c | 67 include/linux/phy.h|7 4 files changed, 87 insertions(+), 40 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v3 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
Add a resource managed devm_mdiobus_alloc[_size]()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Cc: Florian Fainelli f.faine...@gmail.com
Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
Acked-and-tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Documentation/driver-model/devres.txt |5 +++
drivers/net/phy/mdio_bus.c| 67 +
include/linux/phy.h |7
3 files changed, 79 insertions(+)
diff --git a/Documentation/driver-model/devres.txt
b/Documentation/driver-model/devres.txt
index 4f7897e..c74e044 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -308,3 +308,8 @@ SLAVE DMA ENGINE
SPI
devm_spi_register_master()
+
+MDIO
+ devm_mdiobus_alloc()
+ devm_mdiobus_alloc_size()
+ devm_mdiobus_free()
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..68a9a38 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,73 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
}
EXPORT_SYMBOL(mdiobus_alloc_size);
+static void _devm_mdiobus_free(struct device *dev, void *res)
+{
+ mdiobus_free(*(struct mii_bus **)res);
+}
+
+static int devm_mdiobus_match(struct device *dev, void *res, void *data)
+{
+ struct mii_bus **r = res;
+
+ if (WARN_ON(!r || !*r))
+ return 0;
+
+ return *r == data;
+}
+
+/**
+ * devm_mdiobus_alloc_size - Resource-managed mdiobus_alloc_size()
+ * @dev: Device to allocate mii_bus for
+ * @sizeof_priv: Space to allocate for private structure.
+ *
+ * Managed mdiobus_alloc_size. mii_bus allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an mii_bus allocated with this function needs to be freed separately,
+ * devm_mdiobus_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated mii_bus on success, NULL on failure.
+ */
+struct mii_bus *devm_mdiobus_alloc_size(struct device *dev, int sizeof_priv)
+{
+ struct mii_bus **ptr, *bus;
+
+ ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ /* use raw alloc_dr for kmalloc caller tracing */
+ bus = mdiobus_alloc_size(sizeof_priv);
+ if (bus) {
+ *ptr = bus;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return bus;
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_alloc);
+
+/**
+ * devm_mdiobus_free - Resource-managed mdiobus_free()
+ * @dev: Device this mii_bus belongs to
+ * @bus: the mii_bus associated with the device
+ *
+ * Free mii_bus allocated with devm_mdiobus_alloc_size().
+ */
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
+{
+ int rc;
+
+ rc = devres_release(dev, _devm_mdiobus_free,
+ devm_mdiobus_match, bus);
+ WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_free);
+
/**
* mdiobus_release - mii_bus device release callback
* @d: the target struct device that contains the mii_bus
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 24126c4..d7f5ef8 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -195,6 +195,13 @@ static inline struct mii_bus *mdiobus_alloc(void)
int mdiobus_register(struct mii_bus *bus);
void mdiobus_unregister(struct mii_bus *bus);
void mdiobus_free(struct mii_bus *bus);
+struct mii_bus *devm_mdiobus_alloc_size(struct device *dev, int sizeof_priv);
+static inline struct mii_bus *devm_mdiobus_alloc(struct device *dev)
+{
+ return devm_mdiobus_alloc_size(dev, 0);
+}
+
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);
struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v3 3/4] net: davinci_mdio: drop pinctrl_pm_select_default_state from probe
The default pinctrl state is set by Drivers core now before calling the driver's probe. Hence, it's safe to drop pinctrl_pm_select_default_state() call from Davinci mdio driver probe. Cc: Florian Fainelli f.faine...@gmail.com Cc: Linus Walleij linus.wall...@linaro.org Acked-and-tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/net/ethernet/ti/davinci_mdio.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c index 4b202a2..4757198 100644 --- a/drivers/net/ethernet/ti/davinci_mdio.c +++ b/drivers/net/ethernet/ti/davinci_mdio.c @@ -348,9 +348,6 @@ static int davinci_mdio_probe(struct platform_device *pdev) data-bus-parent = dev; data-bus-priv = data; - /* Select default pin state */ - pinctrl_pm_select_default_state(pdev-dev); - pm_runtime_enable(pdev-dev); pm_runtime_get_sync(pdev-dev); data-clk = devm_clk_get(dev, fck); -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
On 04/18/2014 08:52 PM, Sergei Shtylyov wrote:
On 04/18/2014 09:24 PM, Grygorii Strashko wrote:
Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Cc: Florian Fainelli f.faine...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
[...]
index 76f54b3..6412beb 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
[...]
+/**
+ * devm_mdiobus_alloc - Resource-managed mdiobus_alloc_size()
+ * @dev:Device to allocate mii_bus for
+ * @sizeof_priv:Space to allocate for private structure.
+ *
+ * Managed mdiobus_alloc_size. mii_bus allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an mii_bus allocated with this function needs to be freed
separately,
+ * devm_mdiobus_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated mii_bus on success, NULL on failure.
+ */
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv)
+{
+struct mii_bus **ptr, *bus;
+
+ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr),
+ GFP_KERNEL);
+if (!ptr)
+return NULL;
+
+/* use raw alloc_dr for kmalloc caller tracing */
+bus = mdiobus_alloc_size(sizeof_priv);
Since the wrapped function is called mdiobus_alloc_size(), not
mdiobus_alloc(), perhaps it's better to call the wrapper
devm_mdiobus_alloc_size()?
Agree. I've just sent v3.
Regards,
-grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 0/4] introduce devm_mdiobus_alloc/free and clean up davinci mdio
Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free() to automatically clean up MDIO bus alocations made by MDIO drivers, thus leading to simplified MDIO drivers code. Clean up Davinci MDIO driver and use new devm API. Changes in v2: - minor comments taken into account - additional patches added for cleaning up Davinci MDIO driver Cc: Florian Fainelli f.faine...@gmail.com Grygorii Strashko (4): mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free net: davinci_mdio: use devm_* api net: davinci_mdio: drop pinctrl_pm_select_default_state from probe net: davinci_mdio: simplify IO memory mapping Documentation/driver-model/devres.txt |4 ++ drivers/net/ethernet/ti/davinci_mdio.c | 48 -- drivers/net/phy/mdio_bus.c | 68 include/linux/phy.h|2 + 4 files changed, 82 insertions(+), 40 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Cc: Florian Fainelli f.faine...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Documentation/driver-model/devres.txt |4 ++
drivers/net/phy/mdio_bus.c| 68 +
include/linux/phy.h |2 +
3 files changed, 74 insertions(+)
diff --git a/Documentation/driver-model/devres.txt
b/Documentation/driver-model/devres.txt
index 4f7897e..31aa066 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -308,3 +308,7 @@ SLAVE DMA ENGINE
SPI
devm_spi_register_master()
+
+MDIO
+ devm_mdiobus_alloc()
+ devm_mdiobus_free()
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..6412beb 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
}
EXPORT_SYMBOL(mdiobus_alloc_size);
+static void _devm_mdiobus_free(struct device *dev, void *res)
+{
+ mdiobus_free(*(struct mii_bus **)res);
+}
+
+static int devm_mdiobus_match(struct device *dev, void *res, void *data)
+{
+ struct mii_bus **r = res;
+
+ if (WARN_ON(!r || !*r))
+ return 0;
+
+ return *r == data;
+}
+
+/**
+ * devm_mdiobus_alloc - Resource-managed mdiobus_alloc_size()
+ * @dev: Device to allocate mii_bus for
+ * @sizeof_priv: Space to allocate for private structure.
+ *
+ * Managed mdiobus_alloc_size. mii_bus allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an mii_bus allocated with this function needs to be freed separately,
+ * devm_mdiobus_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated mii_bus on success, NULL on failure.
+ */
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv)
+{
+ struct mii_bus **ptr, *bus;
+
+ ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ /* use raw alloc_dr for kmalloc caller tracing */
+ bus = mdiobus_alloc_size(sizeof_priv);
+ if (bus) {
+ *ptr = bus;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return bus;
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_alloc);
+
+/**
+ * devm_mdiobus_free - Resource-managed mdiobus_free()
+ * @dev: Device this mii_bus belongs to
+ * @bus: the mii_bus associated with the device
+ *
+ * Free mii_bus allocated with devm_mdiobus_alloc().
+ */
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
+{
+ int rc;
+
+ rc = devres_release(dev, _devm_mdiobus_free,
+ devm_mdiobus_match, bus);
+ WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_free);
+
/**
* mdiobus_release - mii_bus device release callback
* @d: the target struct device that contains the mii_bus
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 24126c4..2238bea 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -195,6 +195,8 @@ static inline struct mii_bus *mdiobus_alloc(void)
int mdiobus_register(struct mii_bus *bus);
void mdiobus_unregister(struct mii_bus *bus);
void mdiobus_free(struct mii_bus *bus);
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv);
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);
struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 2/4] net: davinci_mdio: use devm_* api
Use devm_* API for memory allocation and to get device's clock
to simplify driver's code.
Cc: Florian Fainelli f.faine...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/net/ethernet/ti/davinci_mdio.c | 24 +---
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 0cca9de..eda4946 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -321,15 +321,14 @@ static int davinci_mdio_probe(struct platform_device
*pdev)
struct phy_device *phy;
int ret, addr;
- data = kzalloc(sizeof(*data), GFP_KERNEL);
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
- data-bus = mdiobus_alloc();
+ data-bus = devm_mdiobus_alloc(dev, 0);
if (!data-bus) {
dev_err(dev, failed to alloc mii bus\n);
- ret = -ENOMEM;
- goto bail_out;
+ return -ENOMEM;
}
if (dev-of_node) {
@@ -354,7 +353,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
pm_runtime_enable(pdev-dev);
pm_runtime_get_sync(pdev-dev);
- data-clk = clk_get(pdev-dev, fck);
+ data-clk = devm_clk_get(dev, fck);
if (IS_ERR(data-clk)) {
dev_err(dev, failed to get device clock\n);
ret = PTR_ERR(data-clk);
@@ -406,16 +405,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
return 0;
bail_out:
- if (data-bus)
- mdiobus_free(data-bus);
-
- if (data-clk)
- clk_put(data-clk);
pm_runtime_put_sync(pdev-dev);
pm_runtime_disable(pdev-dev);
- kfree(data);
-
return ret;
}
@@ -423,18 +415,12 @@ static int davinci_mdio_remove(struct platform_device
*pdev)
{
struct davinci_mdio_data *data = platform_get_drvdata(pdev);
- if (data-bus) {
+ if (data-bus)
mdiobus_unregister(data-bus);
- mdiobus_free(data-bus);
- }
- if (data-clk)
- clk_put(data-clk);
pm_runtime_put_sync(pdev-dev);
pm_runtime_disable(pdev-dev);
- kfree(data);
-
return 0;
}
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 4/4] net: davinci_mdio: simplify IO memory mapping
Simplify IO memory mapping by using devm_ioremap_resource()
which will do all errors handling and reporting for us.
Cc: Florian Fainelli f.faine...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/net/ethernet/ti/davinci_mdio.c | 21 +++--
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 98d53a3..87e9f20 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -363,24 +363,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
spin_lock_init(data-lock);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(dev, could not find register map resource\n);
- ret = -ENOENT;
- goto bail_out;
- }
-
- res = devm_request_mem_region(dev, res-start, resource_size(res),
- dev_name(dev));
- if (!res) {
- dev_err(dev, could not allocate register map resource\n);
- ret = -ENXIO;
- goto bail_out;
- }
-
- data-regs = devm_ioremap_nocache(dev, res-start, resource_size(res));
- if (!data-regs) {
- dev_err(dev, could not map mdio registers\n);
- ret = -ENOMEM;
+ data-regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(data-regs)) {
+ ret = PTR_ERR(data-regs);
goto bail_out;
}
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 3/4] net: davinci_mdio: drop pinctrl_pm_select_default_state from probe
The default pinctrl state is set by Drivers core now before calling the driver's probe. Hence, it's safe to drop pinctrl_pm_select_default_state() call from Davinci mdio driver probe. Cc: Florian Fainelli f.faine...@gmail.com Cs: Linus Walleij linus.wall...@linaro.org Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/net/ethernet/ti/davinci_mdio.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c index eda4946..98d53a3 100644 --- a/drivers/net/ethernet/ti/davinci_mdio.c +++ b/drivers/net/ethernet/ti/davinci_mdio.c @@ -348,9 +348,6 @@ static int davinci_mdio_probe(struct platform_device *pdev) data-bus-parent = dev; data-bus-priv = data; - /* Select default pin state */ - pinctrl_pm_select_default_state(pdev-dev); - pm_runtime_enable(pdev-dev); pm_runtime_get_sync(pdev-dev); data-clk = devm_clk_get(dev, fck); -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free
Hi Florian, On 04/11/2014 07:55 AM, Florian Fainelli wrote: Hi Grygorii, 2014-04-04 6:40 GMT-07:00 Grygorii Strashko grygorii.stras...@ti.com: Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free() to automatically clean up MDIO bus alocations made by MDIO drivers, thus leading to simplified MDIO drivers code. Update Davinci MDIO driver ss example of new devm APIs usage. This does look good at first glance. net-next is currently closed at the moment, so this will have to be merged later. Thanks. It can wait for 3.16, so I'll update resend after rc1. I have few more patches for davinci_mdio.c, so my intention here was to check if I can base them on top of new API or not :) At some point, we might also want to handle the mdio_bus irq array, as that one is also usually dynamically allocated. Maybe we could just do a static irq[PHY_MAX_ADDR] allocation, 32 times the size of an integer might not be worth a potential leak. It sounds good, but first of all irq field of mii_bus structure has to be made private. And drivers have to use getter/setters to access it - then its type can be changes simply and safely. By the way, mdiobus_register() can be handled using DEVM approach too, but it will a bit more complex. [...] Regards, - grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
On 04/04/2014 04:15 PM, Sergei Shtylyov wrote:
Hello.
On 04-04-2014 17:40, Grygorii Strashko wrote:
Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
[...]
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..fdcd6d1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
}
EXPORT_SYMBOL(mdiobus_alloc_size);
+static void _devm_mdiobus_free(struct device *dev, void *res)
+{
+mdiobus_free(*(struct mii_bus **)res);
+}
+
+static int devm_mdiobus_match(struct device *dev, void *res, void *data)
+{
+struct mii_bus **r = res;
Please insert an empty line after declaration.
+if (!r || !*r) {
+WARN_ON(!r || !*r);
Hm, why we need the duplicate check This condition is always true.
It can be replaced with:
if (WARN_ON(!r || !*r))
return 0;
I will wait for additional comments, then resend.
Regards,
- grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free
Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free() to automatically clean up MDIO bus alocations made by MDIO drivers, thus leading to simplified MDIO drivers code. Update Davinci MDIO driver ss example of new devm APIs usage. Grygorii Strashko (2): mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free net: davinci_mdio: use devm_* api Documentation/driver-model/devres.txt |4 ++ drivers/net/ethernet/ti/davinci_mdio.c | 21 ++ drivers/net/phy/mdio_bus.c | 68 include/linux/phy.h|2 + 4 files changed, 78 insertions(+), 17 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 2/2] net: davinci_mdio: use devm_* api
Use devm_* API for memory allocation and to get device's clock
to simplify driver's code.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/net/ethernet/ti/davinci_mdio.c | 21 -
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 0cca9de..f0f7128 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -321,15 +321,14 @@ static int davinci_mdio_probe(struct platform_device
*pdev)
struct phy_device *phy;
int ret, addr;
- data = kzalloc(sizeof(*data), GFP_KERNEL);
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
- data-bus = mdiobus_alloc();
+ data-bus = devm_mdiobus_alloc(dev, 0);
if (!data-bus) {
dev_err(dev, failed to alloc mii bus\n);
- ret = -ENOMEM;
- goto bail_out;
+ return -ENOMEM;
}
if (dev-of_node) {
@@ -354,7 +353,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
pm_runtime_enable(pdev-dev);
pm_runtime_get_sync(pdev-dev);
- data-clk = clk_get(pdev-dev, fck);
+ data-clk = devm_clk_get(dev, fck);
if (IS_ERR(data-clk)) {
dev_err(dev, failed to get device clock\n);
ret = PTR_ERR(data-clk);
@@ -406,16 +405,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
return 0;
bail_out:
- if (data-bus)
- mdiobus_free(data-bus);
-
- if (data-clk)
- clk_put(data-clk);
pm_runtime_put_sync(pdev-dev);
pm_runtime_disable(pdev-dev);
- kfree(data);
-
return ret;
}
@@ -425,16 +417,11 @@ static int davinci_mdio_remove(struct platform_device
*pdev)
if (data-bus) {
mdiobus_unregister(data-bus);
- mdiobus_free(data-bus);
}
- if (data-clk)
- clk_put(data-clk);
pm_runtime_put_sync(pdev-dev);
pm_runtime_disable(pdev-dev);
- kfree(data);
-
return 0;
}
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Documentation/driver-model/devres.txt |4 ++
drivers/net/phy/mdio_bus.c| 68 +
include/linux/phy.h |2 +
3 files changed, 74 insertions(+)
diff --git a/Documentation/driver-model/devres.txt
b/Documentation/driver-model/devres.txt
index 4f7897e..31aa066 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -308,3 +308,7 @@ SLAVE DMA ENGINE
SPI
devm_spi_register_master()
+
+MDIO
+ devm_mdiobus_alloc()
+ devm_mdiobus_free()
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..fdcd6d1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
}
EXPORT_SYMBOL(mdiobus_alloc_size);
+static void _devm_mdiobus_free(struct device *dev, void *res)
+{
+ mdiobus_free(*(struct mii_bus **)res);
+}
+
+static int devm_mdiobus_match(struct device *dev, void *res, void *data)
+{
+ struct mii_bus **r = res;
+ if (!r || !*r) {
+ WARN_ON(!r || !*r);
+ return 0;
+ }
+ return *r == data;
+}
+
+/**
+ * devm_mdiobus_alloc - Resource-managed mdiobus_alloc_size()
+ * @dev: Device to allocate mii_bus for
+ * @sizeof_priv: Space to allocate for private structure.
+ *
+ * Managed mdiobus_alloc_size. mii_bus allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an mii_bus allocated with this function needs to be freed separately,
+ * devm_mdiobus_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated mii_bus on success, NULL on failure.
+ */
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv)
+{
+ struct mii_bus **ptr, *bus;
+
+ ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ /* use raw alloc_dr for kmalloc caller tracing */
+ bus = mdiobus_alloc_size(sizeof_priv);
+ if (bus) {
+ *ptr = bus;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return bus;
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_alloc);
+
+/**
+ * devm_mdiobus_free - Resource-managed mdiobus_free()
+ * @dev: Device this mii_bus belongs to
+ * @bus: the mii_bus associated with the device
+ *
+ * Free mii_bus allocated with devm_mdiobus_alloc().
+ */
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
+{
+ int rc;
+
+ rc = devres_release(dev, _devm_mdiobus_free,
+ devm_mdiobus_match, bus);
+ WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_free);
+
/**
* mdiobus_release - mii_bus device release callback
* @d: the target struct device that contains the mii_bus
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 24126c4..2238bea 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -195,6 +195,8 @@ static inline struct mii_bus *mdiobus_alloc(void)
int mdiobus_register(struct mii_bus *bus);
void mdiobus_unregister(struct mii_bus *bus);
void mdiobus_free(struct mii_bus *bus);
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv);
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);
struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] gpio: davinci: fix gpio selection for OF
On 03/17/2014 03:29 PM, Sekhar Nori wrote: On Friday 14 March 2014 04:32 PM, Linus Walleij wrote: On Fri, Mar 14, 2014 at 11:08 AM, Alexander Holler hol...@ahsoftware.de wrote: Am 11.03.2014 11:15, schrieb Linus Walleij: On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler hol...@ahsoftware.de wrote: The driver missed an of_xlate function to translate gpio numbers as found in the DT to the correct chip and number. While there I've set #gpio_cells to a fixed value of 2. I've used gpio-pxa.c as template for those changes and tested my changes successfully on a da850 board using entries for gpio-leds in a DT. So I didn't reinvent the wheel but just copied and tested stuff. Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa. Signed-off-by: Alexander Holler hol...@ahsoftware.de Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com This v2 version applied, thanks! Thanks, but actually that should have been a fix for 3.14 with which the OF functionality for davinci gpio gets introduced. I assum with the patch in for-next, 3.14 will appear with that functionality broken and it will become a candidate for -stable. I just get the impression that DT support for DaVinci in v3.14 is so risky and unstable that noone except those implementing it (i.e. you) is really using it, is that correct? In that case it is hardly a fix that we need to rush out to the entire world. One thing to note is that this driver is used by keystone too and all its users are DT-only. Although I do not see any in-kernel DT GPIO users even there. I can confirm the patch does not break my gpiolib based test module (test with and without DT), but then it did not have an issue even before. The issues isn't observed on Keystone2 as it has 32 gpios as of now, also GPIO support for keystone is going to be added in 3.15 (including DT changes). Regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] gpio: davinci: fix gpio selection for OF
On 03/05/2014 12:06 AM, Alexander Holler wrote:
The driver missed an of_xlate function to translate gpio numbers
as found in the DT to the correct chip and number.
While there I've set #gpio_cells to a fixed value of 2.
I've used gpio-pxa.c as template for those changes and tested my changes
successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
reinvent the wheel but just copied and tested stuff.
Thanks to Grygorii Strashko for the hint of the existing code in gpio-pxa.
Signed-off-by: Alexander Holler hol...@ahsoftware.de
---
drivers/gpio/gpio-davinci.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..79f45c4 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -44,6 +44,9 @@ struct davinci_gpio_regs {
static void __iomem *gpio_base;
+static struct davinci_gpio_controller *davinci_gpio_chips;
+static int davinci_last_gpio;
+
static struct davinci_gpio_regs __iomem *gpio2regs(unsigned gpio)
{
void __iomem *ptr;
@@ -172,6 +175,24 @@ of_err:
return NULL;
}
+#ifdef CONFIG_OF_GPIO
+static int davinci_gpio_of_xlate(struct gpio_chip *gc,
+const struct of_phandle_args *gpiospec,
+u32 *flags)
+{
+ if (gpiospec-args[0] davinci_last_gpio)
+ return -EINVAL;
+
+ if (gc != davinci_gpio_chips[gpiospec-args[0] / 32].chip)
+ return -EINVAL;
+
+ if (flags)
+ *flags = gpiospec-args[1];
Just one question here - Could we use gpio_chip-dev and hence, drop
static variables (davinci_gpio_chips, davinci_last_gpio)?
The Davinci device holds davinci_gpio_platform_data
in dev-platform_data and pointer on davinci_gpio_controller array
in dev-p-driver_data.
And looks like, dev_get_platdata(gc-dev) and
dev_get_drvdata(gc-dev) can be used here.
+
+ return gpiospec-args[0] % 32;
+}
+#endif
+
static int davinci_gpio_probe(struct platform_device *pdev)
{
int i, base;
@@ -236,6 +257,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
chips[i].chip.ngpio = 32;
add herechips[i].chip.dev = dev;
#ifdef CONFIG_OF_GPIO
+ chips[i].chip.of_gpio_n_cells = 2;
+ chips[i].chip.of_xlate = davinci_gpio_of_xlate;
chips[i].chip.of_node = dev-of_node;
#endif
spin_lock_init(chips[i].lock);
@@ -251,6 +274,10 @@ static int davinci_gpio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, chips);
davinci_gpio_irq_setup(pdev);
+
+ davinci_gpio_chips = chips;
+ davinci_last_gpio = ngpio;
+
return 0;
}
regards,
-grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] gpio: davinci: fix gpio selection for OF
Hi Alexander,
On 03/05/2014 01:21 PM, Alexander Holler wrote:
The driver missed an of_xlate function to translate gpio numbers
as found in the DT to the correct chip and number.
While there I've set #gpio_cells to a fixed value of 2.
I've used gpio-pxa.c as template for those changes and tested my changes
successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
reinvent the wheel but just copied and tested stuff.
Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
Signed-off-by: Alexander Holler hol...@ahsoftware.de
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
Looks good to me now. Thanks.
---
drivers/gpio/gpio-davinci.c | 24
1 file changed, 24 insertions(+)
Changes in v2: replaced static variables by indirections.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..92574a0 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -172,6 +172,27 @@ of_err:
return NULL;
}
+#ifdef CONFIG_OF_GPIO
+static int davinci_gpio_of_xlate(struct gpio_chip *gc,
+const struct of_phandle_args *gpiospec,
+u32 *flags)
+{
+ struct davinci_gpio_controller *chips = dev_get_drvdata(gc-dev);
+ struct davinci_gpio_platform_data *pdata = dev_get_platdata(gc-dev);
+
+ if (gpiospec-args[0] pdata-ngpio)
+ return -EINVAL;
+
+ if (gc != chips[gpiospec-args[0] / 32].chip)
+ return -EINVAL;
+
+ if (flags)
+ *flags = gpiospec-args[1];
+
+ return gpiospec-args[0] % 32;
+}
+#endif
+
static int davinci_gpio_probe(struct platform_device *pdev)
{
int i, base;
@@ -236,6 +257,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
chips[i].chip.ngpio = 32;
#ifdef CONFIG_OF_GPIO
+ chips[i].chip.of_gpio_n_cells = 2;
+ chips[i].chip.of_xlate = davinci_gpio_of_xlate;
+ chips[i].chip.dev = dev;
chips[i].chip.of_node = dev-of_node;
#endif
spin_lock_init(chips[i].lock);
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RESEND][PATCH v4] gpio: davinci: reuse for keystone soc
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
- rebased on top of
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
branch: devel
top commit: ef70bbe gpio: make gpiod_direction_output take a logical value
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 48
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..d0f135d 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -37,6 +37,8 @@ struct davinci_gpio_regs {
u32 intstat;
};
+typedef struct irq_chip *(*gpio_get_irq_chip_cb_t)(unsigned int irq);
+
#define BINTEN 0x8 /* GPIO Interrupt Per-Bank Enable Register */
#define chip2controller(chip) \
@@ -413,6 +415,26 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +455,18 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ gpio_get_irq_chip_cb_t gpio_get_irq_chip;
+
+ /*
+* Use davinci_gpio_get_irq_chip by default to handle non DT cases
+*/
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = (gpio_get_irq_chip_cb_t)match-data;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -489,8 +523,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
* IRQ mux conflicts; gpio_irq_type_unbanked() is only for GPIOs.
*/
if (pdata-gpio_unbanked) {
- static struct irq_chip_type gpio_unbanked;
-
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
chips[0].gpio_irq = bank_irq;
@@ -499,10 +531,9 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* AINTC handles mask/unmask; GPIO handles triggering */
irq = bank_irq;
- gpio_unbanked = *container_of(irq_get_chip(irq),
- struct irq_chip_type, chip);
- gpio_unbanked.chip.name = GPIO-AINTC;
- gpio_unbanked.chip.irq_set_type = gpio_irq_type_unbanked;
+ irq_chip = gpio_get_irq_chip(irq);
+ irq_chip-name = GPIO-AINTC;
+ irq_chip-irq_set_type = gpio_irq_type_unbanked;
/* default trigger: both edges */
g = gpio2regs(0);
@@ -511,7 +542,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio pdata-gpio_unbanked; gpio++, irq++) {
- irq_set_chip(irq, gpio_unbanked.chip);
+ irq_set_chip(irq, irq_chip
[PATCH v4] gpio: davinci: reuse for keystone soc
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Changes in v4:
- rebased on top of v3.14 +
[patch] gpio: davinci: signedness bug in davinci_gpio_irq_setup()
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 48
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index b0e98d3..5c97f61 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -37,6 +37,8 @@ struct davinci_gpio_regs {
u32 intstat;
};
+typedef struct irq_chip *(*gpio_get_irq_chip_cb_t)(unsigned int irq);
+
#define BINTEN 0x8 /* GPIO Interrupt Per-Bank Enable Register */
#define chip2controller(chip) \
@@ -413,6 +415,26 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -434,6 +456,18 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
int irq;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ gpio_get_irq_chip_cb_t gpio_get_irq_chip;
+
+ /*
+* Use davinci_gpio_get_irq_chip by default to handle non DT cases
+*/
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = (gpio_get_irq_chip_cb_t)match-data;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -490,8 +524,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
* IRQ mux conflicts; gpio_irq_type_unbanked() is only for GPIOs.
*/
if (pdata-gpio_unbanked) {
- static struct irq_chip_type gpio_unbanked;
-
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
chips[0].gpio_irq = bank_irq;
@@ -500,10 +532,9 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* AINTC handles mask/unmask; GPIO handles triggering */
irq = bank_irq;
- gpio_unbanked = *container_of(irq_get_chip(irq),
- struct irq_chip_type, chip);
- gpio_unbanked.chip.name = GPIO-AINTC;
- gpio_unbanked.chip.irq_set_type = gpio_irq_type_unbanked;
+ irq_chip = gpio_get_irq_chip(irq);
+ irq_chip-name = GPIO-AINTC;
+ irq_chip-irq_set_type = gpio_irq_type_unbanked;
/* default trigger: both edges */
g = gpio2regs(0);
@@ -512,7 +543,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio pdata-gpio_unbanked; gpio++, irq++) {
- irq_set_chip(irq, gpio_unbanked.chip);
+ irq_set_chip(irq, irq_chip);
irq_set_handler_data(irq, chips[gpio / 32]);
irq_set_status_flags(irq
Re: [patch] gpio: davinci: signedness bug in davinci_gpio_irq_setup()
Hi Linus, Sekhar,
On 01/15/2014 09:21 AM, Linus Walleij wrote:
On Thu, Jan 9, 2014 at 6:28 AM, Dan Carpenter dan.carpen...@oracle.com
wrote:
irq needs to be signed for the error handling to work.
Fixes: 6075a8b2b6c3 ('gpio: davinci: don't create irq_domain in case of
unbanked irqs')
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f12b7f..b0e98d379217 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -423,7 +423,7 @@ static const struct irq_domain_ops davinci_gpio_irq_ops
= {
static int davinci_gpio_irq_setup(struct platform_device *pdev)
{
- unsignedgpio, irq, bank;
+ unsignedgpio, bank;
struct clk *clk;
u32 binten = 0;
unsignedngpio, bank_irq;
@@ -433,6 +433,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ int irq;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
Acked-by: Linus Walleij linus.wall...@linaro.org
This merge window the DaVinci GPIO changes are queued by the DaVinci
maintainers (this patch does not even apply to my tree) so DaVinci guys:
please pick up this patch.
This patch is not in 3.14 yet. Are there any chances to have it in 3.14,
as it introduces merge conflicts with future patches?
Also, this is a fix.
Regards,
-grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 2/2] gpio: davinci: reuse for Keystone SoC
Hi Rob,
On 12/24/2013 01:41 PM, Grygorii Strashko wrote:
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Sekhar Nori nsek...@ti.com
Cc: devicet...@vger.kernel.org
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 48
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
Do you agree with this bindings changes?
They are very simple
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..d0f135d 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -37,6 +37,8 @@ struct davinci_gpio_regs {
u32 intstat;
};
+typedef struct irq_chip *(*gpio_get_irq_chip_cb_t)(unsigned int irq);
+
#define BINTEN0x8 /* GPIO Interrupt Per-Bank Enable Register */
#define chip2controller(chip) \
@@ -413,6 +415,26 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +455,18 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ gpio_get_irq_chip_cb_t gpio_get_irq_chip;
+
+ /*
+* Use davinci_gpio_get_irq_chip by default to handle non DT cases
+*/
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = (gpio_get_irq_chip_cb_t)match-data;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -489,8 +523,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
* IRQ mux conflicts; gpio_irq_type_unbanked() is only for GPIOs.
*/
if (pdata-gpio_unbanked) {
- static struct irq_chip_type gpio_unbanked;
-
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
chips[0].gpio_irq = bank_irq;
@@ -499,10 +531,9 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* AINTC handles mask/unmask; GPIO handles triggering */
irq = bank_irq;
- gpio_unbanked = *container_of(irq_get_chip(irq),
- struct irq_chip_type, chip);
- gpio_unbanked.chip.name = GPIO-AINTC;
- gpio_unbanked.chip.irq_set_type = gpio_irq_type_unbanked;
+ irq_chip = gpio_get_irq_chip(irq);
+ irq_chip-name = GPIO-AINTC;
+ irq_chip-irq_set_type = gpio_irq_type_unbanked;
/* default trigger: both edges */
g = gpio2regs(0);
@@ -511,7 +542,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio pdata-gpio_unbanked; gpio++, irq++) {
- irq_set_chip(irq, gpio_unbanked.chip
[PATCH v3 2/2] gpio: davinci: reuse for Keystone SoC
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Sekhar Nori nsek...@ti.com
Cc: devicet...@vger.kernel.org
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 48
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..d0f135d 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -37,6 +37,8 @@ struct davinci_gpio_regs {
u32 intstat;
};
+typedef struct irq_chip *(*gpio_get_irq_chip_cb_t)(unsigned int irq);
+
#define BINTEN 0x8 /* GPIO Interrupt Per-Bank Enable Register */
#define chip2controller(chip) \
@@ -413,6 +415,26 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +455,18 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ gpio_get_irq_chip_cb_t gpio_get_irq_chip;
+
+ /*
+* Use davinci_gpio_get_irq_chip by default to handle non DT cases
+*/
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = (gpio_get_irq_chip_cb_t)match-data;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -489,8 +523,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
* IRQ mux conflicts; gpio_irq_type_unbanked() is only for GPIOs.
*/
if (pdata-gpio_unbanked) {
- static struct irq_chip_type gpio_unbanked;
-
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
chips[0].gpio_irq = bank_irq;
@@ -499,10 +531,9 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* AINTC handles mask/unmask; GPIO handles triggering */
irq = bank_irq;
- gpio_unbanked = *container_of(irq_get_chip(irq),
- struct irq_chip_type, chip);
- gpio_unbanked.chip.name = GPIO-AINTC;
- gpio_unbanked.chip.irq_set_type = gpio_irq_type_unbanked;
+ irq_chip = gpio_get_irq_chip(irq);
+ irq_chip-name = GPIO-AINTC;
+ irq_chip-irq_set_type = gpio_irq_type_unbanked;
/* default trigger: both edges */
g = gpio2regs(0);
@@ -511,7 +542,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio pdata-gpio_unbanked; gpio++, irq++) {
- irq_set_chip(irq, gpio_unbanked.chip);
+ irq_set_chip(irq, irq_chip);
irq_set_handler_data(irq, chips[gpio / 32
Re: [PATCH v2 1/2] gpio: davinci: don't create irq_domain in case of unbanked irqs
On 12/22/2013 05:52 PM, Sekhar Nori wrote:
On Wednesday 18 December 2013 03:37 PM, Grygorii Strashko wrote:
The system may crash if:
- there are more then 1 bank
s/then/than
- unbanked irqs are enabled
- someone will call gpio_to_irq() for GPIO from bank2 or above
Hence, fix it by not creating irq_domain if unbanked irqs are enabled
and correct gpio_to_irq_banked() to handle this properly.
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Sekhar Nori nsek...@ti.com
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/gpio/gpio-davinci.c | 34 +++---
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 5d163c0..1b33806 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -351,7 +351,10 @@ static int gpio_to_irq_banked(struct gpio_chip *chip,
unsigned offset)
{
struct davinci_gpio_controller *d = chip2controller(chip);
- return irq_create_mapping(d-irq_domain, d-chip.base + offset);
+ if (d-irq_domain)
+ return irq_create_mapping(d-irq_domain, d-chip.base + offset);
+ else
+ return -ENXIO;
}
static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
@@ -429,7 +432,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
- struct irq_domain *irq_domain;
+ struct irq_domain *irq_domain = NULL;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -453,18 +456,20 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
clk_prepare_enable(clk);
- irq = irq_alloc_descs(-1, 0, ngpio, 0);
- if (irq 0) {
- dev_err(dev, Couldn't allocate IRQ numbers\n);
- return irq;
- }
+ if (!pdata-gpio_unbanked) {
+ irq = irq_alloc_descs(-1, 0, ngpio, 0);
+ if (irq 0) {
+ dev_err(dev, Couldn't allocate IRQ numbers\n);
+ return -ENODEV;
The code was correct before moving. Any objections to doing
return irq;
instead?
This is mistake. I'll update. Thanks
Thanks,
Sekhar
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 2/2] gpio: davinci: reuse for Keystone SoC
On 12/22/2013 05:41 PM, Sekhar Nori wrote:
On Wednesday 18 December 2013 03:37 PM, Grygorii Strashko wrote:
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Sekhar Nori nsek...@ti.com
Cc: devicet...@vger.kernel.org
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 46
2 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 1b33806..38741cc 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -413,6 +413,26 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +453,18 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ struct irq_chip *(*gpio_get_irq_chip)(unsigned int irq);
+
+ /*
+* Use davinci_gpio_get_irq_chip by default to handle non DT cases
+*/
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = match-data;
This produces a sparse warning:
CHECK drivers/gpio/gpio-davinci.c
drivers/gpio/gpio-davinci.c:467:35: warning: incorrect type in assignment
(different modifiers)
drivers/gpio/gpio-davinci.c:467:35:expected struct irq_chip *( *[assigned]
gpio_get_irq_chip )( ... )
drivers/gpio/gpio-davinci.c:467:35:got void const *const data
I'll fix it, thanks.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 0/2] gpio: davinci: reuse for keystone arch
This series is intended to update Davinci GPIO driver and reuse it for Keystone SoCs, because Keystone uses the similar GPIO IP like Davinci. Keystone GPIO IP: supports: - up to 32 GPIO lines; - only unbanked irqs; See Documentation: Keystone - http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf This series based on: https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v3.14/gpio Changes in v2: - minor comments applied, no functional changes v1: https://lkml.org/lkml/2013/12/12/366 Cc: Linus Walleij linus.wall...@linaro.org Cc: Alexandre Courbot gnu...@gmail.com Cc: Sekhar Nori nsek...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Grygorii Strashko (2): gpio: davinci: don't create irq_domain in case of unbanked irqs gpio: davinci: reuse for Keystone SoC .../devicetree/bindings/gpio/gpio-davinci.txt |4 +- drivers/gpio/gpio-davinci.c| 80 ++-- 2 files changed, 59 insertions(+), 25 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2 2/2] gpio: davinci: reuse for Keystone SoC
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Sekhar Nori nsek...@ti.com
Cc: devicet...@vger.kernel.org
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 46
2 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 1b33806..38741cc 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -413,6 +413,26 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +453,18 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ struct irq_chip *(*gpio_get_irq_chip)(unsigned int irq);
+
+ /*
+* Use davinci_gpio_get_irq_chip by default to handle non DT cases
+*/
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = match-data;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -489,8 +521,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
* IRQ mux conflicts; gpio_irq_type_unbanked() is only for GPIOs.
*/
if (pdata-gpio_unbanked) {
- static struct irq_chip_type gpio_unbanked;
-
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
chips[0].gpio_irq = bank_irq;
@@ -499,10 +529,9 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* AINTC handles mask/unmask; GPIO handles triggering */
irq = bank_irq;
- gpio_unbanked = *container_of(irq_get_chip(irq),
- struct irq_chip_type, chip);
- gpio_unbanked.chip.name = GPIO-AINTC;
- gpio_unbanked.chip.irq_set_type = gpio_irq_type_unbanked;
+ irq_chip = gpio_get_irq_chip(irq);
+ irq_chip-name = GPIO-AINTC;
+ irq_chip-irq_set_type = gpio_irq_type_unbanked;
/* default trigger: both edges */
g = gpio2regs(0);
@@ -511,7 +540,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio pdata-gpio_unbanked; gpio++, irq++) {
- irq_set_chip(irq, gpio_unbanked.chip);
+ irq_set_chip(irq, irq_chip);
irq_set_handler_data(irq, chips[gpio / 32]);
irq_set_status_flags(irq, IRQ_TYPE_EDGE_BOTH);
}
@@ -554,7 +583,8 @@ done:
#if IS_ENABLED(CONFIG_OF)
static const struct of_device_id davinci_gpio_ids[] = {
- { .compatible = ti,dm6441-gpio, },
+ { .compatible = ti,keystone-gpio
[PATCH v2 1/2] gpio: davinci: don't create irq_domain in case of unbanked irqs
The system may crash if:
- there are more then 1 bank
- unbanked irqs are enabled
- someone will call gpio_to_irq() for GPIO from bank2 or above
Hence, fix it by not creating irq_domain if unbanked irqs are enabled
and correct gpio_to_irq_banked() to handle this properly.
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Sekhar Nori nsek...@ti.com
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/gpio/gpio-davinci.c | 34 +++---
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 5d163c0..1b33806 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -351,7 +351,10 @@ static int gpio_to_irq_banked(struct gpio_chip *chip,
unsigned offset)
{
struct davinci_gpio_controller *d = chip2controller(chip);
- return irq_create_mapping(d-irq_domain, d-chip.base + offset);
+ if (d-irq_domain)
+ return irq_create_mapping(d-irq_domain, d-chip.base + offset);
+ else
+ return -ENXIO;
}
static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
@@ -429,7 +432,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
- struct irq_domain *irq_domain;
+ struct irq_domain *irq_domain = NULL;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -453,18 +456,20 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
clk_prepare_enable(clk);
- irq = irq_alloc_descs(-1, 0, ngpio, 0);
- if (irq 0) {
- dev_err(dev, Couldn't allocate IRQ numbers\n);
- return irq;
- }
+ if (!pdata-gpio_unbanked) {
+ irq = irq_alloc_descs(-1, 0, ngpio, 0);
+ if (irq 0) {
+ dev_err(dev, Couldn't allocate IRQ numbers\n);
+ return -ENODEV;
+ }
- irq_domain = irq_domain_add_legacy(NULL, ngpio, irq, 0,
- davinci_gpio_irq_ops,
- chips);
- if (!irq_domain) {
- dev_err(dev, Couldn't register an IRQ domain\n);
- return -ENODEV;
+ irq_domain = irq_domain_add_legacy(NULL, ngpio, irq, 0,
+ davinci_gpio_irq_ops,
+ chips);
+ if (!irq_domain) {
+ dev_err(dev, Couldn't register an IRQ domain\n);
+ return -ENODEV;
+ }
}
/*
@@ -475,8 +480,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
*/
for (gpio = 0, bank = 0; gpio ngpio; bank++, gpio += 32) {
chips[bank].chip.to_irq = gpio_to_irq_banked;
- if (!pdata-gpio_unbanked)
- chips[bank].irq_domain = irq_domain;
+ chips[bank].irq_domain = irq_domain;
}
/*
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/2] gpio: davinci: reuse for keystone arch
On 12/15/2013 03:54 PM, Sekhar Nori wrote: On Sunday 15 December 2013 07:20 PM, Sekhar Nori wrote: On Sunday 15 December 2013 12:41 AM, Santosh Shilimkar wrote: Linus, Sekhar, On Thursday 12 December 2013 01:12 PM, Grygorii Strashko wrote: This series is intended to update Davinci GPIO driver and reuse it for Keystone SoCs, because Keystone uses the similar GPIO IP like Davinci. Keystone GPIO IP: supports: - up to 32 GPIO lines; - only unbanked irqs; See Documentation: Keystone - http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf This series depends on: [1] [PATCH 1/2] gpio: davinci: Fix a check for unbanked gpio https://lkml.org/lkml/2013/11/8/22 [2] [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement https://www.mail-archive.com/devicetree@vger.kernel.org/msg05970.html [3] gpio: davinci: get rid of DAVINCI_N_GPIO https://lkml.org/lkml/2013/11/26/405 [4] gpio: introduce GPIO_DAVINCI kconfig option https://lkml.org/lkml/2013/11/26/435 [5] gpio: davinci: use chained_irq_enter/chained_irq_exit API https://lkml.org/lkml/2013/11/26/428 To handle all dependencies, I've created a branch where I collected all ready to merge patches (all acks added in patches) and this series: - https://github.com/grygoriyS/linux.git - branch: keystone-master-gpio-for-next Can one of you pull all these patches ? So I went through my backlog and queued all that I think is ready. Here is the branch. Let me know if there is anything else missing. Looks like everything are in place. Thanks. Forgot to mention that I have not been able to test them today though. They will hit linux-next only after I have been able to test them and I send a pull request to arm-soc or Linus W. Regards, -grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/2] gpio: davinci: reuse for Keystone SoC
On 12/16/2013 09:29 AM, Alexandre Courbot wrote:
On Fri, Dec 13, 2013 at 3:12 AM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
CC: Linus Walleij linus.wall...@linaro.org
CC: Sekhar Nori nsek...@ti.com
CC: Santosh Shilimkar santosh.shilim...@ti.com
CC: devicet...@vger.kernel.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 49 +++-
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 73f65ca..3e44e0c 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -413,6 +413,27 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ pr_err(keystone_gpio_get_irq_chip\n);
Do you intend this pr_err() to remain here?
No, thanks - will remove.
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +454,15 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ struct irq_chip *(*gpio_get_irq_chip)(unsigned int irq);
+
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = match-data;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -442,7 +472,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
bank_irq = res-start;
-
if (!bank_irq) {
dev_err(dev, Invalid IRQ resource\n);
return -ENODEV;
@@ -484,25 +513,22 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
/*
-* AINTC can handle direct/unbanked IRQs for GPIOs, with the GPIO
+* INTC can handle direct/unbanked IRQs for GPIOs, with the GPIO
* controller only handling trigger modes. We currently assume no
* IRQ mux conflicts; gpio_irq_type_unbanked() is only for GPIOs.
*/
if (pdata-gpio_unbanked) {
- static struct irq_chip_type gpio_unbanked;
-
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
chips[0].gpio_irq = bank_irq;
chips[0].gpio_unbanked = pdata-gpio_unbanked;
binten = BIT(0);
- /* AINTC handles mask/unmask; GPIO handles triggering */
+ /* INTC handles mask/unmask; GPIO handles triggering */
irq = bank_irq;
- gpio_unbanked = *container_of(irq_get_chip(irq),
- struct irq_chip_type, chip);
- gpio_unbanked.chip.name = GPIO-AINTC;
- gpio_unbanked.chip.irq_set_type = gpio_irq_type_unbanked;
+ irq_chip = gpio_get_irq_chip(irq);
+ irq_chip-name = GPIO-AINTC;
According to the other renamings I see in this file, shouldn't the
string also be changed to GPIO-INTC?
Right. Will change, it should
Re: [PATCH 2/2] gpio: davinci: reuse for Keystone SoC
On 12/16/2013 06:38 PM, Santosh Shilimkar wrote:
On Thursday 12 December 2013 01:12 PM, Grygorii Strashko wrote:
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
CC: Linus Walleij linus.wall...@linaro.org
CC: Sekhar Nori nsek...@ti.com
CC: Santosh Shilimkar santosh.shilim...@ti.com
CC: devicet...@vger.kernel.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 49 +++-
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 73f65ca..3e44e0c 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -413,6 +413,27 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ pr_err(keystone_gpio_get_irq_chip\n);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +454,15 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ struct irq_chip *(*gpio_get_irq_chip)(unsigned int irq);
+
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = match-data;
and if the DT node is not populated ?
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -442,7 +472,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
bank_irq = res-start;
-
stray change..
if (!bank_irq) {
dev_err(dev, Invalid IRQ resource\n);
return -ENODEV;
@@ -484,25 +513,22 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
/*
-* AINTC can handle direct/unbanked IRQs for GPIOs, with the GPIO
+* INTC can handle direct/unbanked IRQs for GPIOs, with the GPIO
So you want to have a generic name here. If you are changing it, change it
across the driver in a separate patch or leave it as is...
I don't think it matters much..
Uh. Ok. I'll drop renaming.
With those comments addressed, you can add my ack.
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 0/2] gpio: davinci: reuse for keystone arch
This series is intended to update Davinci GPIO driver and reuse
it for Keystone SoCs, because Keystone uses the similar GPIO IP like Davinci.
Keystone GPIO IP: supports:
- up to 32 GPIO lines;
- only unbanked irqs;
See Documentation:
Keystone - http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
This series depends on:
[1] [PATCH 1/2] gpio: davinci: Fix a check for unbanked gpio
https://lkml.org/lkml/2013/11/8/22
[2] [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement
https://www.mail-archive.com/devicetree@vger.kernel.org/msg05970.html
[3] gpio: davinci: get rid of DAVINCI_N_GPIO
https://lkml.org/lkml/2013/11/26/405
[4] gpio: introduce GPIO_DAVINCI kconfig option
https://lkml.org/lkml/2013/11/26/435
[5] gpio: davinci: use chained_irq_enter/chained_irq_exit API
https://lkml.org/lkml/2013/11/26/428
To handle all dependencies, I've created a branch where I collected all
ready to merge patches (all acks added in patches) and this series:
- https://github.com/grygoriyS/linux.git
- branch: keystone-master-gpio-for-next
The keystone-master-gpio-for-next based on:
https://git.kernel.org/cgit/linux/kernel/git/ssantosh/linux-keystone.git
branch: keystone/master
+ where I've merged in:
https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git
branch: fixes
List of commits:
6fb66de gpio: davinci: reuse for Keystone SoC
c0c5422 gpio: davinci: don't create irq_domain in case of unbanked irqs
df9f7bc gpio: davinci: use chained_irq_enter/chained_irq_exit API
6230c92 gpio: davinci: introduce GPIO_DAVINCI kconfig option
8b0c1a8 gpio: davinci: get rid of DAVINCI_N_GPIO
5eb07f7 gpio: davinci: add OF support
df3e59d gpio: davinci: remove unused variable intc_irq_num
38804b6 gpio: davinci: converts to use irqdomain
94eac24 gpio: davinci: use {readl|writel}_relaxed() instead of __raw_*
CC: Linus Walleij linus.wall...@linaro.org
CC: Sekhar Nori nsek...@ti.com
CC: Santosh Shilimkar santosh.shilim...@ti.com
Grygorii Strashko (2):
gpio: davinci: don't create irq_domain in case of unbanked irqs
gpio: davinci: reuse for Keystone SoC
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 83 ++--
2 files changed, 59 insertions(+), 28 deletions(-)
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 2/2] gpio: davinci: reuse for Keystone SoC
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone taking into
account that Keystone contains ARM GIC IRQ controller which
is implemented using IRQ Chip.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
CC: Linus Walleij linus.wall...@linaro.org
CC: Sekhar Nori nsek...@ti.com
CC: Santosh Shilimkar santosh.shilim...@ti.com
CC: devicet...@vger.kernel.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 49 +++-
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 73f65ca..3e44e0c 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -413,6 +413,27 @@ static const struct irq_domain_ops davinci_gpio_irq_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
+static struct irq_chip *davinci_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip_type gpio_unbanked;
+
+ gpio_unbanked = *container_of(irq_get_chip(irq),
+ struct irq_chip_type, chip);
+
+ return gpio_unbanked.chip;
+};
+
+static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
+{
+ static struct irq_chip gpio_unbanked;
+
+ gpio_unbanked = *irq_get_chip(irq);
+ pr_err(keystone_gpio_get_irq_chip\n);
+ return gpio_unbanked;
+};
+
+static const struct of_device_id davinci_gpio_ids[];
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -433,6 +454,15 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
struct irq_domain *irq_domain = NULL;
+ const struct of_device_id *match;
+ struct irq_chip *irq_chip;
+ struct irq_chip *(*gpio_get_irq_chip)(unsigned int irq);
+
+ gpio_get_irq_chip = davinci_gpio_get_irq_chip;
+ match = of_match_device(of_match_ptr(davinci_gpio_ids),
+ dev);
+ if (match)
+ gpio_get_irq_chip = match-data;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -442,7 +472,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
bank_irq = res-start;
-
if (!bank_irq) {
dev_err(dev, Invalid IRQ resource\n);
return -ENODEV;
@@ -484,25 +513,22 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
/*
-* AINTC can handle direct/unbanked IRQs for GPIOs, with the GPIO
+* INTC can handle direct/unbanked IRQs for GPIOs, with the GPIO
* controller only handling trigger modes. We currently assume no
* IRQ mux conflicts; gpio_irq_type_unbanked() is only for GPIOs.
*/
if (pdata-gpio_unbanked) {
- static struct irq_chip_type gpio_unbanked;
-
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
chips[0].gpio_irq = bank_irq;
chips[0].gpio_unbanked = pdata-gpio_unbanked;
binten = BIT(0);
- /* AINTC handles mask/unmask; GPIO handles triggering */
+ /* INTC handles mask/unmask; GPIO handles triggering */
irq = bank_irq;
- gpio_unbanked = *container_of(irq_get_chip(irq),
- struct irq_chip_type, chip);
- gpio_unbanked.chip.name = GPIO-AINTC;
- gpio_unbanked.chip.irq_set_type = gpio_irq_type_unbanked;
+ irq_chip = gpio_get_irq_chip(irq);
+ irq_chip-name = GPIO-AINTC;
+ irq_chip-irq_set_type = gpio_irq_type_unbanked;
/* default trigger: both edges */
g = gpio2regs(0);
@@ -511,7 +537,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio pdata-gpio_unbanked; gpio++, irq
[PATCH 1/2] gpio: davinci: don't create irq_domain in case of unbanked irqs
The system may crash if:
- there are more then 1 bank
- unbanked irqs are enabled
- someone will call gpio_to_irq() for GPIO from bank2 or above
Hence, fix it by not creating irq_domain if unbanked irqs are enabled
and correct gpio_to_irq_banked() to handle this properly.
CC: Linus Walleij linus.wall...@linaro.org
CC: Sekhar Nori nsek...@ti.com
CC: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/gpio/gpio-davinci.c | 34 +++---
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 9379b4d..73f65ca 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -351,7 +351,10 @@ static int gpio_to_irq_banked(struct gpio_chip *chip,
unsigned offset)
{
struct davinci_gpio_controller *d = chip2controller(chip);
- return irq_create_mapping(d-irq_domain, d-chip.base + offset);
+ if (d-irq_domain)
+ return irq_create_mapping(d-irq_domain, d-chip.base + offset);
+ else
+ return -ENXIO;
}
static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
@@ -429,7 +432,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
- struct irq_domain *irq_domain;
+ struct irq_domain *irq_domain = NULL;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -453,18 +456,20 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
clk_prepare_enable(clk);
- irq = irq_alloc_descs(-1, 0, ngpio, 0);
- if (irq 0) {
- dev_err(dev, Couldn't allocate IRQ numbers\n);
- return -ENODEV;
- }
+ if (!pdata-gpio_unbanked) {
+ irq = irq_alloc_descs(-1, 0, ngpio, 0);
+ if (irq 0) {
+ dev_err(dev, Couldn't allocate IRQ numbers\n);
+ return -ENODEV;
+ }
- irq_domain = irq_domain_add_legacy(NULL, ngpio, irq, 0,
- davinci_gpio_irq_ops,
- chips);
- if (!irq_domain) {
- dev_err(dev, Couldn't register an IRQ domain\n);
- return -ENODEV;
+ irq_domain = irq_domain_add_legacy(NULL, ngpio, irq, 0,
+ davinci_gpio_irq_ops,
+ chips);
+ if (!irq_domain) {
+ dev_err(dev, Couldn't register an IRQ domain\n);
+ return -ENODEV;
+ }
}
/*
@@ -475,8 +480,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
*/
for (gpio = 0, bank = 0; gpio ngpio; bank++, gpio += 32) {
chips[bank].chip.to_irq = gpio_to_irq_banked;
- if (!pdata-gpio_unbanked)
- chips[bank].irq_domain = irq_domain;
+ chips[bank].irq_domain = irq_domain;
}
/*
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC v1 0/9] gpio: davinci: reuse for keystone arch
Hi All, On 11/29/2013 10:37 AM, Linus Walleij wrote: On Tue, Nov 26, 2013 at 8:40 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: [1] Depends on patch: [PATCH 1/2] gpio: davinci: Fix a check for unbanked gpio https://lkml.org/lkml/2013/11/8/22 [2] and depends on series from Prabhakar Lad: [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement https://www.mail-archive.com/devicetree@vger.kernel.org/msg05970.html So this needs to go through the same tree as these patches. And I suggested that Sekhar queue them and either take them directly up to ARM SoC or send me a pull request to take it through the GPIO tree. I'm going to go in and review these now... Thanks for you review. I'll update, rebase and repost patches once the Depends On series will be accepted. Regards, - grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt new file mode 100644 index 000..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +Davinci GPIO controller bindings + +Required Properties: +- compatible: should be ti,dm6441-gpio + +- reg: Physical base address of the controller and the size of memory mapped + registers. + +- gpio-controller : Marks the device node as a gpio controller. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt + line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Few things here: - The mixed banked/unbanked functionality has never been supported before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Do you have any thoughts about how it can be done in a simpler way? Actually, the same was proposed by Linus, but we've tried avoid such huge rework - by switching to one irq_domain per all banks for example. Regards, - grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC v1 0/9] gpio: davinci: reuse for keystone arch
This series is intended to update Davinci GPIO driver and reuse it for Keystone SoCs, because Keystone uses the similar GPIO IP like Davinci. Keystone GPIO IP: supports: - up to 32 GPIO lines; - only unbanked irqs; See Documentation: Keystone - http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf [1] Depends on patch: [PATCH 1/2] gpio: davinci: Fix a check for unbanked gpio https://lkml.org/lkml/2013/11/8/22 [2] and depends on series from Prabhakar Lad: [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement https://www.mail-archive.com/devicetree@vger.kernel.org/msg05970.html Based on: git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git branch: keystone/master This series has been marked as RFC because it's based on an unfinished series of patches [2], but with hope that it will help to reach final decisions and satisfy all interested parties. Grygorii Strashko (9): gpio: davinci: get rid of DAVINCI_N_GPIO gpio: introduce GPIO_DAVINCI kconfig option gpio: davinci: use chained_irq_enter/chained_irq_exit API gpio: davinci: make IRQ initialization soc specific gpio: davinci: reuse for Keystone SoC arm: dts: keystone: add GPIO device entry ARM: keystone_defconfig: enable gpio support arm: dts: keystone-evm: add LEDs supports ARM: keystone_defconfig: enable LED support .../devicetree/bindings/gpio/gpio-davinci.txt |4 +- arch/arm/boot/dts/k2hk-evm.dts | 23 ++ arch/arm/boot/dts/keystone.dtsi| 45 +++ arch/arm/configs/keystone_defconfig| 11 + drivers/gpio/Kconfig |7 + drivers/gpio/Makefile |2 +- drivers/gpio/gpio-davinci.c| 306 ++-- 7 files changed, 305 insertions(+), 93 deletions(-) -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC v1 1/9] gpio: davinci: get rid of DAVINCI_N_GPIO
Since Davinci GPIO driver is moved to support gpiolib it has to use ARCH_NR_GPIOS (can be configured using CONFIG_ARCH_NR_GPIO Kconfig option) configuration instead of any mach/platform specific options. Hence, replace private DAVINCI_N_GPIO with common ARCH_NR_GPIOS. This is safe because default value for ARCH_NR_GPIOS=256 and maximum number of supported GPIOs for Davinci is DAVINCI_N_GPIO=144. More over, this is one of steps to re-use Davinci GPIO driver by other mach/platform. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/gpio/gpio-davinci.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c index 5bdd9f8..70b5f2f 100644 --- a/drivers/gpio/gpio-davinci.c +++ b/drivers/gpio/gpio-davinci.c @@ -200,8 +200,8 @@ static int davinci_gpio_probe(struct platform_device *pdev) return -EINVAL; } - if (WARN_ON(DAVINCI_N_GPIO ngpio)) - ngpio = DAVINCI_N_GPIO; + if (WARN_ON(ARCH_NR_GPIOS ngpio)) + ngpio = ARCH_NR_GPIOS; chips = devm_kzalloc(dev, ngpio * sizeof(struct davinci_gpio_controller), -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC v1 9/9] ARM: keystone_defconfig: enable LED support
The Keystone GPIO functionality is ready for use, so LED support can be enabled in config. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- arch/arm/configs/keystone_defconfig |8 1 file changed, 8 insertions(+) diff --git a/arch/arm/configs/keystone_defconfig b/arch/arm/configs/keystone_defconfig index e911f7f..fb1eb8a 100644 --- a/arch/arm/configs/keystone_defconfig +++ b/arch/arm/configs/keystone_defconfig @@ -161,3 +161,11 @@ CONFIG_CRYPTO_USER_API_SKCIPHER=y CONFIG_GPIOLIB=y CONFIG_GPIO_SYSFS=y CONFIG_GPIO_DAVINCI=y +CONFIG_LEDS_CLASS=y +CONFIG_NEW_LEDS=y +CONFIG_LEDS_GPIO=y +CONFIG_LEDS_TRIGGERS=y +CONFIG_LEDS_TRIGGER_ONESHOT=y +CONFIG_LEDS_TRIGGER_HEARTBEAT=y +CONFIG_LEDS_TRIGGER_BACKLIGHT=y +CONFIG_LEDS_TRIGGER_GPIO=y -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC v1 5/9] gpio: davinci: reuse for Keystone SoC
The similar GPIO HW block is used by keystone SoCs as
in Davinci SoCs.
Hence, reuse Davinci GPIO driver for Keystone.
Documentation:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt |4 +-
drivers/gpio/gpio-davinci.c| 55
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..4ce9862 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,7 @@
-Davinci GPIO controller bindings
+Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be ti,dm6441-gpio
+- compatible: should be ti,dm6441-gpio, ti,keystone-gpio
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 6a48bf8..545f25c 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -542,11 +542,65 @@ static int davinci_gpio_banked_irq_init(struct
platform_device *pdev)
return 0;
}
+static int keystone_gpio_unbanked_irq_init(struct platform_device *pdev)
+{
+ int base_irq, irq;
+ unsigned gpio, ngpio;
+ struct davinci_gpio_regs __iomem *g;
+ struct device *dev = pdev-dev;
+ struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
+ struct davinci_gpio_platform_data *pdata = dev-platform_data;
+
+ static struct irq_chip gpio_unbanked;
+
+ if (pdata-gpio_unbanked chips[0].chip.ngpio) {
+ dev_err(dev, Invalid IRQ configuration\n);
+ return -EINVAL;
+ }
+
+ ngpio = pdata-ngpio;
+
+ base_irq = platform_get_irq(pdev, 0);
+ if (base_irq = 0) {
+ dev_err(dev, Invalid first banked IRQ number %d\n, base_irq);
+ return base_irq 0 ? base_irq : -EINVAL;
+ }
+
+ /* pass bank 0 GPIO IRQs to GIC */
+ chips[0].chip.to_irq = gpio_to_irq_unbanked;
+ chips[0].gpio_irq = base_irq;
+ chips[0].gpio_unbanked = pdata-gpio_unbanked;
+
+ /* GIC handles mask/unmask; GPIO handles triggering */
+ gpio_unbanked = *irq_get_chip(base_irq);
+ gpio_unbanked.name = GPIO-GIC;
+ gpio_unbanked.irq_set_type = gpio_irq_type_unbanked;
+
+ /* default trigger: both edges */
+ g = gpio2regs(0);
+ writel(~0, g-set_falling);
+ writel(~0, g-set_rising);
+
+ irq = base_irq;
+ /* set the direct IRQs up to use that irqchip */
+ for (gpio = 0; gpio pdata-gpio_unbanked; gpio++, irq++) {
+ irq_set_chip(irq, gpio_unbanked);
+ irq_set_handler_data(irq, chips[gpio / 32]);
+ irq_set_status_flags(irq, IRQ_TYPE_EDGE_BOTH);
+ }
+
+ return 0;
+};
+
static const struct davinci_gpio_init_data davinci_gpio_pdata = {
.unbanked_irq_init = davinci_gpio_unbanked_irq_init,
.banked_irq_init = davinci_gpio_banked_irq_init,
};
+static const struct davinci_gpio_init_data keystone_gpio_pdata = {
+ .unbanked_irq_init = keystone_gpio_unbanked_irq_init,
+};
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -622,6 +676,7 @@ done:
#if IS_ENABLED(CONFIG_OF)
static const struct of_device_id davinci_gpio_ids[] = {
{ .compatible = ti,dm6441-gpio, davinci_gpio_pdata},
+ { .compatible = ti,keystone-gpio, keystone_gpio_pdata},
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC v1 6/9] arm: dts: keystone: add GPIO device entry
This patch adds Keystone GPIO IP device definitions in DT which supports
up to 32 GPIO lines and each GPIO line can be configured as separate
interrupt source (so called unbanked IRQ).
For more information see:
http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
arch/arm/boot/dts/keystone.dtsi | 45 +++
1 file changed, 45 insertions(+)
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index f6d6d9e..6e93bf5 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -7,6 +7,7 @@
*/
#include dt-bindings/interrupt-controller/arm-gic.h
+#include dt-bindings/gpio/gpio.h
#include skeleton.dtsi
@@ -181,5 +182,49 @@
interrupts = GIC_SPI 300 IRQ_TYPE_EDGE_RISING;
clocks = clkspi;
};
+
+ gpio0: gpio@260bf00 {
+ compatible = ti,keystone-gpio;
+ reg = 0x0260bf00 0x100;
+ gpio-controller;
+ #gpio-cells = 2;
+ /* HW Interrupts mapped to GPIO pins */
+ interrupts = GIC_SPI 120 IRQ_TYPE_EDGE_RISING
+ GIC_SPI 121
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 122
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 123
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 124
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 125
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 126
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 127
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 128
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 129
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 130
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 131
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 132
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 133
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 134
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 135
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 136
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 137
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 138
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 139
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 140
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 141
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 142
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 143
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 144
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 145
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 146
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 147
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 148
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 149
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 150
IRQ_TYPE_EDGE_RISING
+ GIC_SPI 151
IRQ_TYPE_EDGE_RISING;
+ clocks = clkgpio;
+ clock-names = gpio;
+ ti,ngpio = 32;
+ ti,davinci-gpio-unbanked = 32;
+ };
};
};
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC v1 7/9] ARM: keystone_defconfig: enable gpio support
Enable enable GPIO support for Keystone by setting CONFIG_GPIOLIB and CONFIG_GPIO_DAVINCI options in keystone_defconfig. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- arch/arm/configs/keystone_defconfig |3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/configs/keystone_defconfig b/arch/arm/configs/keystone_defconfig index 9943e5d..e911f7f 100644 --- a/arch/arm/configs/keystone_defconfig +++ b/arch/arm/configs/keystone_defconfig @@ -158,3 +158,6 @@ CONFIG_CRYPTO_DES=y CONFIG_CRYPTO_ANSI_CPRNG=y CONFIG_CRYPTO_USER_API_HASH=y CONFIG_CRYPTO_USER_API_SKCIPHER=y +CONFIG_GPIOLIB=y +CONFIG_GPIO_SYSFS=y +CONFIG_GPIO_DAVINCI=y -- 1.7.9.5 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC v1 8/9] arm: dts: keystone-evm: add LEDs supports
Keystone EVMK2HX supports 4 debug LEDs controlled by GPIO lines as
following (active level is high);
DBG_D1 green gpio12
DBG_D1 red gpio13
DBG_D1 blue gpio14
DBG_D1 blue gpio15
For more information see schematics:
http://wfcache.advantech.com/www/support/TI-EVM/download/Schematics/PDF/K2H_K2EVM-HK_SCH_A102_Rev1_0.pdf
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
arch/arm/boot/dts/k2hk-evm.dts | 23 +++
1 file changed, 23 insertions(+)
diff --git a/arch/arm/boot/dts/k2hk-evm.dts b/arch/arm/boot/dts/k2hk-evm.dts
index 15b3a95..07bf1e9 100644
--- a/arch/arm/boot/dts/k2hk-evm.dts
+++ b/arch/arm/boot/dts/k2hk-evm.dts
@@ -52,4 +52,27 @@
};
};
};
+
+ leds {
+ compatible = gpio-leds;
+ debug1_1 {
+ label = keystone:green:debug1;
+ gpios = gpio0 12 GPIO_ACTIVE_HIGH; /* 12 */
+ };
+
+ debug1_2 {
+ label = keystone:red:debug1;
+ gpios = gpio0 13 GPIO_ACTIVE_HIGH; /* 13 */
+ };
+
+ debug2 {
+ label = keystone:blue:debug2;
+ gpios = gpio0 14 GPIO_ACTIVE_HIGH; /* 14 */
+ };
+
+ debug3 {
+ label = keystone:blue:debug3;
+ gpios = gpio0 15 GPIO_ACTIVE_HIGH; /* 15 */
+ };
+ };
};
--
1.7.9.5
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On 11/26/2013 07:12 PM, Sekhar Nori wrote: On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt new file mode 100644 index 000..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +Davinci GPIO controller bindings + +Required Properties: +- compatible: should be ti,dm6441-gpio + +- reg: Physical base address of the controller and the size of memory mapped + registers. + +- gpio-controller : Marks the device node as a gpio controller. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt +line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Okay. That's why I inserted that comment in parenthesis :) Few things here: - The mixed banked/unbanked functionality has never been supported before. True. I actually misread the driver before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. Well the only common register I can see is BINTEN - bank interrupt enable. This register can simply be initialized to enable interrupts from all banks possible as until the rising and falling edge triggers are programmed, there wont be any actual interrupts generated. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Well, I certainly don't think its a complete driver re-write. It will take a bit of effort agreed, but I think the driver will also come out a lot cleaner. Honestly, I am not so much worried about the kernel code here. Its the bindings I am worried about. Once the bindings go in assuming there will never be banked and unbanked GPIO IRQs on the same SoC, changing them to do something else will be very painful with the need to keep backward compatibility and support for both semantics. That said, because there is no present hardware which needs both banked and unbanked at the same time, I wont press for this to be done endlessly. Do you have any thoughts about how it can be done in a simpler way? I don't know if there is a simpler way, but I don't think there is too much effort too. I leave it to those implementing it though. Oh. I see no problem to implement it for DT, but this change require to convert one device to the tree of devices: GPIO controller |- GPIO bank1 ... |- GPIO bankX And that's will need to be handled somehow from platform code (which is non-DT and which I can't verify and which I don't want to touch actually ;). Actually, the same was proposed by Linus, but we've tried avoid
Re: [PATCH v5 3/7] gpio: davinci: use irqdomain
Hi Linus,
On 11/18/2013 03:11 PM, Linus Walleij wrote:
On Fri, Nov 8, 2013 at 11:11 AM, Prabhakar Lad
prabhakar.cse...@gmail.com wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
This patch converts the davinci gpio driver to use irqdomain
support.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
(...)
@@ -282,8 +283,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
desc-irq_data.chip-irq_ack(desc-irq_data);
while (1) {
u32 status;
- int n;
- int res;
+ int bit;
Why is this an int? I think u8 would suffice.
/* now demux them to the right lowlevel handler */
- n = d-irq_base;
- if (irq 1) {
- n += 16;
- status = 16;
- }
-
while (status) {
- res = ffs(status);
- n += res;
- generic_handle_irq(n - 1);
- status = res;
+ bit = __ffs(status);
+ status = ~(1 bit);
+ generic_handle_irq(irq_find_mapping(d-irq_domain,
+ bit));
This is a nice hunk of the patch.
I would use linux/bitops.h and do:
status = ~BIT(bit);
@@ -313,10 +307,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip,
unsigned offset)
{
struct davinci_gpio_controller *d = chip2controller(chip);
- if (d-irq_base = 0)
- return d-irq_base + offset;
- else
- return -ENODEV;
+ return irq_create_mapping(d-irq_domain, offset);
}
I think we recently established that map creating cannot be done
in gpio_to_irq* functions as that will not work properly if you request
an IRQ from device tree without first obtaining the IRQ from the GPIO
number with this function.
Why? Could you point on corresponding thread, pls?
Current call tree is:
irq_create_of_mapping()
|-hwirq = omain-ops-xlate()
|-irq_create_mapping(domain, hwirq)
Instead call irq_create_mapping() on *all* used IRQ lines in the
probe function and use irq_find_mapping() here too.
+ for (gpio = 0, bank = 0; gpio ngpio; bank++, bank_irq++, gpio += 16) {
/* disabled by default, enabled only as needed */
g = gpio2regs(gpio);
writel(~0, g-clr_falling);
@@ -467,14 +483,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
*/
irq_set_handler_data(bank_irq, chips[gpio / 32]);
So for example here you could call iurq_create_mapping();
Also: please write a patch that marks the IRQ lines.
Call gpio_lock_as_irq(*gpio_chip, offset); in the
irqchip startup/shutdown functions. (Can be a separate
patch.)
It seems, some misunderstanding is here. So I'd like to clarify few
points and would be very appreciate for your comments:
1) This patch itself will work unless we switch to DT (as in the
following patch)
2) After this patch the following object structure will be created
during Davinci GPIO driver initialization (DA850 has 144 IRQ lines):
- gpio_chip0(0..31)
|- irq_domain1
- gpio_chip1(32..63)
|- irq_domain2
- gpio_chip2(64..95)
|- irq_domain3
- gpio_chip3(96..127)
|- irq_domain4
- gpio_chip4(128..143)
|- irq_domain5
3) But in case of DT only one GPIO controller node will be created
Example:
gpio: gpio@1e26000 {
compatible = ti,dm6441-gpio;
gpio-controller;
reg = 0x226000 0x1000;
interrupt-parent = intc;
interrupts = 42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
50 IRQ_TYPE_EDGE_BOTH;
interrupt-controller;
#interrupt-cells = 2;
ti,ngpio = 144;
ti,davinci-gpio-unbanked = 0;
}
4) As result, to make GPIO bindings/mappings work - we'll need to
implement .of_xlate() callback per GPIO chip, which will provide us with
valid valid gpio_chip and offset of gpio inside chip
(It was discussed before and supposed to be done as next step).
For example, gpio_chip3 and offset=15 should be selected:
devA {
gpios = gpio 111 GPIO_ACTIVE_HIGH;
}
5) What should be done to make GPIO IRQ bindings/mappings work?
Example:
devB {
interrupt-parent = gpio;
interrupts = 111 IRQ_TYPE_EDGE_BOTH;
}
Should we implement one IRQ domain per all GPIO chips (per GPIO controller)?
Thanks.
Regards,
-Grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/2] DaVinci: GPIO: fixes
Hi Sekhar, On 11/12/2013 08:18 AM, Sekhar Nori wrote: On Monday 11 November 2013 09:13 PM, Grygorii Strashko wrote: Hi Sekhar, Prabhakar Lad On 11/08/2013 08:45 AM, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch series fixes gpio driver regestration and offset check for unbanked gpio. Lad, Prabhakar (2): gpio: davinci: Fix a check for unbanked gpio ARM: davinci: Fix number of resources passed to davinci_gpio_register() call I've verified both patches - looks ok. Patch 1 - restores unbanked GPIO IRQs functionality Patch 2 - allows to register GPIO Platform devices and boot without issues. Could we move forward with these patches? Prabhakar, both patches look good to me. Thanks for the fixes. Also Grygorii, have you actually verified that the unbanked IRQ support works? Only DM365 uses it. Yes, I've verified it, but on Keystone which has similar GPIO IP which supports only unbanked IRQs. What do I add for you? Tested-by: or Acked-by: Reported-and-Reviewed-and-Tested-by: Grygorii Strashko grygorii.stras...@ti.com :P ^Up to you :), but if I understand things right Acked-by: included all above Regards, - grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v5 5/7] gpio: davinci: add OF support
On 11/08/2013 12:11 PM, Prabhakar Lad wrote:
From: KV Sujith sujit...@ti.com
This patch adds OF parser support for davinci gpio
driver and also appropriate documentation in gpio-davinci.txt
located at Documentation/devicetree/bindings/gpio/.
Signed-off-by: KV Sujith sujit...@ti.com
Signed-off-by: Philip Avinash avinashphi...@ti.com
[prabhakar.cse...@gmail.com: simplified the OF code, removed
unnecessary DT property and also simplified
the commit message]
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt | 39 ++
drivers/gpio/gpio-davinci.c| 54
++--
2 files changed, 90 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
new file mode 100644
index 000..d677bbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,39 @@
+Davinci GPIO controller bindings
+
+Required Properties:
+- compatible: should be ti,dm6441-gpio
+
+- reg: Physical base address of the controller and the size of memory mapped
+ registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- interrupt-parent: phandle of the parent interrupt controller.
+
+- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs
are
+ supported at a time.
+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
interrupt
+ line to processor.
+
+The GPIO controller also acts as an interrupt controller. It uses the default
+two cells specifier as described in Documentation/devicetree/bindings/
+interrupt-controller/interrupts.txt.
+
+Example:
+
+gpio: gpio@1e26000 {
+ compatible = ti,dm6441-gpio;
+ gpio-controller;
+ reg = 0x226000 0x1000;
+ interrupt-parent = intc;
+ interrupts = 42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
+ 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
+ 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
+ 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
+ 50 IRQ_TYPE_EDGE_BOTH;
+ ti,ngpio = 144;
+ ti,davinci-gpio-unbanked = 0;
+};
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index b149239..ed04835 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -17,6 +17,9 @@
#include linux/io.h
#include linux/irq.h
#include linux/irqdomain.h
+#include linux/module.h
+#include linux/of.h
+#include linux/of_device.h
#include linux/platform_device.h
#include linux/platform_data/gpio-davinci.h
@@ -134,6 +137,40 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned
offset, int value)
writel((1 offset), value ? g-set_data : g-clr_data);
}
+static struct davinci_gpio_platform_data *
+davinci_gpio_get_pdata(struct platform_device *pdev)
Minor: usually such functions have _of in their names:
davinci_gpio_of_get_pdata()
+{
+ struct device_node *dn = pdev-dev.of_node;
+ struct davinci_gpio_platform_data *pdata;
+ int ret;
+ u32 val;
+
+ if (!IS_ENABLED(CONFIG_OF) || !pdev-dev.of_node)
+ return pdev-dev.platform_data;
+
+ pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ ret = of_property_read_u32(dn, ti,ngpio, val);
+ if (ret)
+ goto of_err;
+
+ pdata-ngpio = val;
+
+ ret = of_property_read_u32(dn, ti,davinci-gpio-unbanked, val);
+ if (ret)
+ goto of_err;
+
+ pdata-gpio_unbanked = val;
+
+ return pdata;
+
+of_err:
+ dev_err(pdev-dev, Populating pdata from DT failed: err %d\n, ret);
+ return NULL;
+}
+
static int davinci_gpio_probe(struct platform_device *pdev)
{
int i, base;
@@ -144,12 +181,14 @@ static int davinci_gpio_probe(struct platform_device
*pdev)
struct device *dev = pdev-dev;
struct resource *res;
- pdata = dev-platform_data;
+ pdata = davinci_gpio_get_pdata(pdev);
if (!pdata) {
dev_err(dev, No platform data found\n);
return -EINVAL;
}
+ dev-platform_data = pdata;
+
Pls, add following code to GPIO chip initialization:
@@ -233,6 +245,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
chips[i].chip.ngpio = ngpio - base;
if (chips[i].chip.ngpio 32)
chips[i].chip.ngpio = 32;
+#ifdef CONFIG_OF_GPIO
+ chips[i].chip.of_node = dev-of_node;
+#endif
/*
* The gpio banks conceptually expose a segmented bitmap,
* and ngpio is one more
Re: [PATCH v5 5/7] gpio: davinci: add OF support
On 11/11/2013 05:37 PM, Prabhakar Lad wrote:
Hi Grygorii,
Thanks for the review.
On Mon, Nov 11, 2013 at 8:48 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
On 11/08/2013 12:11 PM, Prabhakar Lad wrote:
[Snip]
@@ -134,6 +137,40 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset,
int value)
writel((1 offset), value ? g-set_data : g-clr_data);
}
+static struct davinci_gpio_platform_data *
+davinci_gpio_get_pdata(struct platform_device *pdev)
Minor: usually such functions have _of in their names:
davinci_gpio_of_get_pdata()
Ahh but actual this function is intended to get pdata in both the
cases DT and NON-DT, so kept it generic :)
np
+{
+ struct device_node *dn = pdev-dev.of_node;
+ struct davinci_gpio_platform_data *pdata;
+ int ret;
+ u32 val;
+
+ if (!IS_ENABLED(CONFIG_OF) || !pdev-dev.of_node)
+ return pdev-dev.platform_data;
+
+ pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ ret = of_property_read_u32(dn, ti,ngpio, val);
+ if (ret)
+ goto of_err;
+
+ pdata-ngpio = val;
+
+ ret = of_property_read_u32(dn, ti,davinci-gpio-unbanked, val);
+ if (ret)
+ goto of_err;
+
+ pdata-gpio_unbanked = val;
+
+ return pdata;
+
+of_err:
+ dev_err(pdev-dev, Populating pdata from DT failed: err %d\n, ret);
+ return NULL;
+}
+
static int davinci_gpio_probe(struct platform_device *pdev)
{
int i, base;
@@ -144,12 +181,14 @@ static int davinci_gpio_probe(struct platform_device
*pdev)
struct device *dev = pdev-dev;
struct resource *res;
- pdata = dev-platform_data;
+ pdata = davinci_gpio_get_pdata(pdev);
if (!pdata) {
dev_err(dev, No platform data found\n);
return -EINVAL;
}
+ dev-platform_data = pdata;
+
Pls, add following code to GPIO chip initialization:
@@ -233,6 +245,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
chips[i].chip.ngpio = ngpio - base;
if (chips[i].chip.ngpio 32)
chips[i].chip.ngpio = 32;
+#ifdef CONFIG_OF_GPIO
+ chips[i].chip.of_node = dev-of_node;
+#endif
OK
Regards,
--Prabhakar Lad
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/2] DaVinci: GPIO: fixes
Hi Sekhar, Prabhakar Lad On 11/08/2013 08:45 AM, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch series fixes gpio driver regestration and offset check for unbanked gpio. Lad, Prabhakar (2): gpio: davinci: Fix a check for unbanked gpio ARM: davinci: Fix number of resources passed to davinci_gpio_register() call I've verified both patches - looks ok. Patch 1 - restores unbanked GPIO IRQs functionality Patch 2 - allows to register GPIO Platform devices and boot without issues. Could we move forward with these patches? arch/arm/mach-davinci/dm355.c |2 +- arch/arm/mach-davinci/dm365.c |2 +- arch/arm/mach-davinci/dm644x.c |2 +- arch/arm/mach-davinci/dm646x.c |2 +- drivers/gpio/gpio-davinci.c|4 +++- 5 files changed, 7 insertions(+), 5 deletions(-) ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v5 0/7] gpio: daVinci: cleanup and feature enhancement
Hi Prabhakar Lad, On 11/08/2013 12:11 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch series does the following 1 Ports the driver to use irqdomain. 2 Adds dt binding support for gpio-davinci. 3 Adds DA850 dt support goio. Changes for v5: 1: Fixed review comments pointed by Grygorii. 2: Dropped the fixup patch This patch series is based on following fixup series http://us.generation-nt.com/answer/patch-0-2-davinci-gpio-fixes-help-212978272.html Few comments to this series - the rest looks ok to me. Hope it will pass review of bindings :P KV Sujith (3): gpio: davinci: add OF support ARM: davinci: da850: add GPIO DT node ARM: davinci: da850 evm: add GPIO pinumux entries DT node Lad, Prabhakar (4): gpio: davinci: remove unnecessary printk gpio: davinci: use readl/writel instead of __raw_* gpio: davinci: use irqdomain gpio: davinci: remove unused variable intc_irq_num .../devicetree/bindings/gpio/gpio-davinci.txt | 39 + arch/arm/boot/dts/da850-evm.dts| 20 +++ arch/arm/boot/dts/da850.dtsi | 14 ++ arch/arm/mach-davinci/da830.c |1 - arch/arm/mach-davinci/da850.c |1 - arch/arm/mach-davinci/dm355.c |1 - arch/arm/mach-davinci/dm365.c |1 - arch/arm/mach-davinci/dm644x.c |1 - arch/arm/mach-davinci/dm646x.c |1 - drivers/gpio/gpio-davinci.c| 170 +--- include/linux/platform_data/gpio-davinci.h |3 +- 11 files changed, 185 insertions(+), 67 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt Regards, - grygorii ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v5 1/7] gpio: davinci: remove unnecessary printk
On 11/08/2013 12:11 PM, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
the devm_*() helper prints error messages in case of
errors no need to do the same in the driver.
Pls, drop it - devm_clk_get() doesn't always print error messages
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
drivers/gpio/gpio-davinci.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 84be701..7230c43 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -389,11 +389,9 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
clk = devm_clk_get(dev, gpio);
- if (IS_ERR(clk)) {
- printk(KERN_ERR Error %ld getting gpio clock?\n,
- PTR_ERR(clk));
+ if (IS_ERR(clk))
return PTR_ERR(clk);
- }
+
clk_prepare_enable(clk);
/*
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
Hi Sekhar, Linus,
On 11/06/2013 12:49 PM, Sekhar Nori wrote:
On Wednesday 06 November 2013 03:45 PM, Prabhakar Lad wrote:
Hi Linus,
On Wed, Nov 6, 2013 at 3:26 PM, Linus Walleij linus.wall...@linaro.org wrote:
On Wed, Nov 6, 2013 at 10:33 AM, Prabhakar Lad
prabhakar.cse...@gmail.com wrote:
On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij linus.wall...@linaro.org wrote:
On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
prabhakar.cse...@gmail.com wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
This patch fixes the check for the offset in
gpio_to_irq_unbanked() function.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Is this a regression that should go in right now?
Yes it needs too.
But on top of *what* exactly?
This does not apply to my gpio tree devel branch and
not even on the mainline kernel.
Looks like this needs to go via ARM tree as the earlier
patches have gone via ARM tree itself [1].
If you can ACK it Sekhar can get it in via ARM tree.
The dependent patches are all in linux-next through ARM SoC queued for
v3.13 merge. This fix can either be sent late in merge cycle once Linus
has pulled ARM SoC or after v3.13-rc1 comes out.
Pls, wait a bit - this fix is incomplete :( The below changes have to be
done too:
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 6c90cfb..05dbadb 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -328,7 +328,7 @@ static int gpio_to_irq_unbanked(struct gpio_chip
*chip, unsigned offset)
* can provide direct-mapped IRQs to AINTC (up to 32 GPIOs).
*/
if (offset d-gpio_unbanked)
- return d-gpio_irq + offset;
+ return d-irq_base + offset;
else
return -ENODEV;
}
@@ -341,7 +341,7 @@ static int gpio_irq_type_unbanked(struct irq_data
*data, unsigned trigger)
d = (struct davinci_gpio_controller *)data-handler_data;
g = (struct davinci_gpio_regs __iomem *)d-regs;
- mask = __gpio_mask(data-irq - d-gpio_irq);
+ mask = __gpio_mask(data-irq - d-irq_base);
if (trigger ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
return -EINVAL;
@@ -419,6 +419,8 @@ static int davinci_gpio_irq_setup(struct
platform_device *pdev)
/* pass bank 0 GPIO IRQs to AINTC */
chips[0].chip.to_irq = gpio_to_irq_unbanked;
+ chips[0].irq_base = bank_irq;
+ chips[0].gpio_unbanked = pdata-gpio_unbanked;
binten = BIT(0);
/* AINTC handles mask/unmask; GPIO handles triggering */
diff --git a/include/linux/platform_data/gpio-davinci.h
b/include/linux/platform_data/gpio-davinci.h
index 6efd202..711a002 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -42,7 +42,6 @@ struct davinci_gpio_controller {
void __iomem*clr_data;
void __iomem*in_data;
int gpio_unbanked;
- unsignedgpio_irq;
};
==
Also all places where davinci_gpio_register() is called need to be
updated to use instead of sizeof. Like:
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index ef9ff1f..10334a4 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -906,8 +906,8 @@ static struct davinci_gpio_platform_data
dm355_gpio_platform_data = {
int __init dm355_gpio_register(void)
{
return davinci_gpio_register(dm355_gpio_resources,
-sizeof(dm355_gpio_resources),
-dm355_gpio_platform_data);
+ ARRAY_SIZE(dm355_gpio_resources),
+ dm355_gpio_platform_data);
}
Regards,
- grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 4/6] gpio: davinci: add OF support
On 11/06/2013 12:08 PM, Prabhakar Lad wrote:
Hi Grygorii,
On Tue, Nov 5, 2013 at 10:29 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
On 11/05/2013 10:53 AM, Prabhakar Lad wrote: Hi Grygorii,
Thanks for the review.
On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Hi Prabhakar Lad,
On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
From: KV Sujith sujit...@ti.com
This patch adds OF parser support for davinci gpio
driver and also appropriate documentation in gpio-davinci.txt
located at Documentation/devicetree/bindings/gpio/.
I worry, do we need to have gpio_chip.of_xlate() callback implemented?
I looked for the other OF GPIO implementations with same ngpio
property (marvel, msm) but I don’t see of_xlate() callback implemented.
The question: will below definitions in DT work or not after this series?
Will of_get_gpio()/of_get_named_gpio() work?
Example1 - leds:
leds {
compatible = gpio-leds;
debug0 {
label = green:debug0;
gpios = gpio 29 GPIO_ACTIVE_HIGH;
};
};
Example2 - any dev:
devA {
compatible = devA;
gpios = gpio 120 GPIO_ACTIVE_LOW;
}
Agreed of_get_gpio()/of_get_named_gpio() wont work without
xlate callback implemented, but I think this can be added as a
incremental patch later.
- From one side, Davinci GPIO controller in DT described by one entry
which defines number of supported GPIOs as ti,ngpio = 144;
- From other side, on Linux level more than one gpio_chip objects are
instantiated (one per each 32 GPIO).
How the standard GPIO biding will work in this case? .. And will they?
Linus, I'd very appreciate if you will be able to clarify this point.
Signed-off-by: KV Sujith sujit...@ti.com
Signed-off-by: Philip Avinash avinashphi...@ti.com
[prabhakar.cse...@gmail.com: simplified the OF code, removed
unnecessary DT property and also simplified
the commit message]
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt | 32
drivers/gpio/gpio-davinci.c| 54
++--
2 files changed, 83 insertions(+), 3 deletions(-)
create mode 100644
Documentation/devicetree/bindings/gpio/gpio-davinci.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
new file mode 100644
index 000..55aae1c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,32 @@
+Davinci GPIO controller bindings29
+
+Required Properties:
+- compatible: should be ti,dm6441-gpio
+
+- reg: Physical base address of the controller and the size of memory
mapped
+ registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- interrupts: Array of GPIO interrupt number.
May be meaning of interrupts property need to be extended, because,
as of now, only banked or unbanked IRQs are supported - and not both.
OK
+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
interrupt
+line to processor.
Should interrupt-controller; specifier be added here?
No
So, it would be impossible to map GPIO IRQ to device through DT. Right?
Like:
devX@0 {
compatible = devX;
interrupt-parent = gpio;
interrupts = 50 IRQ_TYPE_EDGE_FALLING; /* gpio line 50 */
};
may be I took you wrong here, the interrupt-controller is inherited
property taken from its parent, so didn’t mention that in the documentation
The GPIO controller uses interrupts form parent controller INTC/GIC
from one side, but from other side it can provide interrupts to its
users.
And as result can be interrupt-controller.
INTC/GIC - GPIO - user
It could work for banked IRQs only now :)
Regards,
--Prabhakar Lad
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 4/6] gpio: davinci: add OF support
On 11/05/2013 10:53 AM, Prabhakar Lad wrote: Hi Grygorii,
Thanks for the review.
On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Hi Prabhakar Lad,
On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
From: KV Sujith sujit...@ti.com
This patch adds OF parser support for davinci gpio
driver and also appropriate documentation in gpio-davinci.txt
located at Documentation/devicetree/bindings/gpio/.
I worry, do we need to have gpio_chip.of_xlate() callback implemented?
I looked for the other OF GPIO implementations with same ngpio
property (marvel, msm) but I don’t see of_xlate() callback implemented.
The question: will below definitions in DT work or not after this series?
Will of_get_gpio()/of_get_named_gpio() work?
Example1 - leds:
leds {
compatible = gpio-leds;
debug0 {
label = green:debug0;
gpios = gpio 29 GPIO_ACTIVE_HIGH;
};
};
Example2 - any dev:
devA {
compatible = devA;
gpios = gpio 120 GPIO_ACTIVE_LOW;
}
- From one side, Davinci GPIO controller in DT described by one entry
which defines number of supported GPIOs as ti,ngpio = 144;
- From other side, on Linux level more than one gpio_chip objects are
instantiated (one per each 32 GPIO).
How the standard GPIO biding will work in this case? .. And will they?
Linus, I'd very appreciate if you will be able to clarify this point.
Signed-off-by: KV Sujith sujit...@ti.com
Signed-off-by: Philip Avinash avinashphi...@ti.com
[prabhakar.cse...@gmail.com: simplified the OF code, removed
unnecessary DT property and also simplified
the commit message]
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
.../devicetree/bindings/gpio/gpio-davinci.txt | 32
drivers/gpio/gpio-davinci.c| 54
++--
2 files changed, 83 insertions(+), 3 deletions(-)
create mode 100644
Documentation/devicetree/bindings/gpio/gpio-davinci.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
new file mode 100644
index 000..55aae1c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,32 @@
+Davinci GPIO controller bindings29
+
+Required Properties:
+- compatible: should be ti,dm6441-gpio
+
+- reg: Physical base address of the controller and the size of memory
mapped
+ registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- interrupts: Array of GPIO interrupt number.
May be meaning of interrupts property need to be extended, because,
as of now, only banked or unbanked IRQs are supported - and not both.
OK
+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an
individual
interrupt
+line to processor.
Should interrupt-controller; specifier be added here?
No
So, it would be impossible to map GPIO IRQ to device through DT. Right?
Like:
devX@0 {
compatible = devX;
interrupt-parent = gpio;
interrupts = 50 IRQ_TYPE_EDGE_FALLING; /* gpio line 50 */
};
+
+Example:
+
+gpio: gpio@1e26000 {
+ compatible = ti,dm6441-gpio;
+ gpio-controller;
+ reg = 0x226000 0x1000;
+ interrupts = 42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
+ 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
+ 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
+ 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
+ 50 IRQ_TYPE_EDGE_BOTH;
+ ti,ngpio = 144;
+ ti,davinci-gpio-irq-base = 101;
^^ Is it still needed?
OOps missed to remove that.
Regards,
-grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 3/6] gpio: davinci: use irqdomain
Hi Prabhakar Lad,
On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
This patch converts the davinci gpio driver to use irqdomain
support.
This patch needs to be splitted in two:
1) add IRQ domain support
2) remove intc_irq_num
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
arch/arm/mach-davinci/da830.c |1 -
arch/arm/mach-davinci/da850.c |1 -
arch/arm/mach-davinci/dm355.c |1 -
arch/arm/mach-davinci/dm365.c |1 -
arch/arm/mach-davinci/dm644x.c |1 -
arch/arm/mach-davinci/dm646x.c |1 -
drivers/gpio/gpio-davinci.c| 49
++--
include/linux/platform_data/gpio-davinci.h |3 +-
8 files changed, 32 insertions(+), 26 deletions(-)
[...]
int __init dm646x_gpio_register(void)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 95c6df1..bcb6d8d 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -16,6 +16,7 @@
#include linux/err.h
#include linux/io.h
#include linux/irq.h
+#include linux/irqdomain.h
#include linux/platform_device.h
#include linux/platform_data/gpio-davinci.h
@@ -292,7 +293,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
__raw_writel(status, g-intstat);
/* now demux them to the right lowlevel handler */
- n = d-irq_base;
+ n = irq_find_mapping(d-irq_domain, 0);
Sorry, but I don't understand why have you used 0 as hwirq?
All below logic may not work in case if we switch to use Linear IRQ domain :(
- irq_create_mapping() may return ANY Linux IRQ number. It means, for
example, for bank0(ngpio=32)[0] it may return Linux_IRQ=200 or 201 or any other.
Also, for bank3(ngpio=16)[0] it may return Linux_IRQ=150, etc.
- More over, if you call irq_create_mapping() 32 times you may NOT get
sequential Linux_IRQ numbers.
So, the better sequence here can be smth. as below
(I can't verify it - my HW support only unbanked IRQs):
if (irq 1)
mask = 16;
while (1) {
u32 status;
int bit;
/* ack any irqs */
status = __raw_readl(g-intstat) mask;
if (!status)
break;
__raw_writel(status, g-intstat);
/* now demux them to the right lowlevel handler */
while (status) {
bit = __ffs(status);
status = ~(1 bit);
generic_handle_irq(irq_find_mapping(d-irq_domain,
bit));
}
}
if (irq 1) {
n += 16;
status = 16;
@@ -313,10 +314,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip,
unsigned offset)
{
struct davinci_gpio_controller *d = chip2controller(chip);
- if (d-irq_base = 0)
- return d-irq_base + offset;
- else
- return -ENODEV;
+ return irq_find_mapping(d-irq_domain, offset);
I think you can use irq_create_mapping() here instead of
irq_find_mapping(), so IRQ will be mapped/allocated on demand.
Also, it seems, above code may crash in case if SoC has 1 GPIO banks and
gpio_unbanked 0 and someone will call gpio_to_irq(31).
}
static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
@@ -373,6 +371,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
struct davinci_gpio_platform_data *pdata = dev-platform_data;
struct davinci_gpio_regs __iomem *g;
+ int gpio_irq = 0;
ngpio = pdata-ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -402,9 +401,15 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
*/
for (gpio = 0, bank = 0; gpio ngpio; bank++, gpio += 32) {
chips[bank].chip.to_irq = gpio_to_irq_banked;
- chips[bank].irq_base = pdata-gpio_unbanked
- ? -EINVAL
- : (pdata-intc_irq_num + gpio);
+ if (!pdata-gpio_unbanked) {
+ chips[bank].irq_domain =
+ irq_domain_add_linear(NULL, 32,
Use chips[i].chip.ngpio here instead of const 32?
+ irq_domain_simple_ops,
Pass davinci_gpio_irq_ops (see below)
+ NULL);
Pass chips[bank] as host_data and use .map() callback (see below)
+
+ if (!chips[bank].irq_domain)
+ return -ENOMEM;
+ }
}
/*
@@ -445,9 +450,7 @@ static int davinci_gpio_irq_setup(struct
