| Message ID | 20171106194618.9421-1-tpiepho@impinj.com |
|---|---|
| Headers | show |
| Series | Fix for imx-spi CS GPIOs | expand |
Hi, On 06.11.2017 20:46, Trent Piepho wrote: > The driver will fail to load if the device tree property for gpio chip > selects is not specified. Change the driver to make the property optional. > Also have the driver respect the "num-cs" property in the standard way. > > This way one can use the native CS lines using standard bindings instead of > being forced to specify a gpio-cs of <0>. > > Since native CS do not work in a commonly useful manner with this > hardware/driver, note in the documention that most users probably should > use GPIO based CS lines rather than native. > > CC: Mark Brown <broonie@kernel.org> > CC: Shawn Guo <shawnguo@kernel.org> > CC: Sascha Hauer <kernel@pengutronix.de> > CC: Fabio Estevam <fabio.estevam@nxp.com> > CC: Oleksij Rempel <o.rempel@pengutronix.de> > Signed-off-by: Trent Piepho <tpiepho@impinj.com> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt | 18 ++++++++++++------ > drivers/spi/spi-imx.c | 18 ++++++++++-------- > 2 files changed, 22 insertions(+), 14 deletions(-) > > diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt > index 5bf13960f7f4..e3c48b20b1a6 100644 > --- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt > +++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt > @@ -12,24 +12,30 @@ Required properties: > - "fsl,imx53-ecspi" for SPI compatible with the one integrated on i.MX53 and later Soc > - reg : Offset and length of the register set for the device > - interrupts : Should contain CSPI/eCSPI interrupt > -- cs-gpios : Specifies the gpio pins to be used for chipselects. > - clocks : Clock specifiers for both ipg and per clocks. > - clock-names : Clock names should include both "ipg" and "per" > See the clock consumer binding, > Documentation/devicetree/bindings/clock/clock-bindings.txt > -- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, > - Documentation/devicetree/bindings/dma/dma.txt > -- dma-names: DMA request names should include "tx" and "rx" if present. > > -Obsolete properties: > -- fsl,spi-num-chipselects : Contains the number of the chipselect > +Recommended properties: > +- cs-gpios : GPIOs to use as chip selects, see spi-bus.txt. While the native chip > +select lines can be used, they appear to always generate a pulse between each > +word of a transfer. Most use cases will require GPIO based chip selects to > +generate a valid transaction. > > Optional properties: > +- num-cs : Number of total chip selects, see spi-bus.txt. > +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, > +Documentation/devicetree/bindings/dma/dma.txt. > +- dma-names: DMA request names, if present, should include "tx" and "rx". > - fsl,spi-rdy-drctl: Integer, representing the value of DRCTL, the register > controlling the SPI_READY handling. Note that to enable the DRCTL consideration, > the SPI_READY mode-flag needs to be set too. > Valid values are: 0 (disabled), 1 (edge-triggered burst) and 2 (level-triggered burst). > > +Obsolete properties: > +- fsl,spi-num-chipselects : Contains the number of the chipselect > + > Example: > > ecspi@70010000 { > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index 301cdb721bad..ad54f8258513 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -1523,6 +1523,7 @@ static int spi_imx_probe(struct platform_device *pdev) > > spi_imx->devtype_data = devtype_data; > > + /* Get number of chip selects, either platform data or OF */ > if (mxc_platform_info) { > master->num_chipselect = mxc_platform_info->num_chipselect; > master->cs_gpios = devm_kzalloc(&master->dev, > @@ -1532,7 +1533,13 @@ static int spi_imx_probe(struct platform_device *pdev) > > for (i = 0; i < master->num_chipselect; i++) > master->cs_gpios[i] = mxc_platform_info->chipselect[i]; > - } > + } else { > + u32 num_cs; > + > + if (!of_property_read_u32(np, "num-cs", &num_cs)) > + master->num_chipselect = num_cs; > + /* If not preset, default value of 1 is used */ > + } > > spi_imx->bitbang.chipselect = spi_imx_chipselect; > spi_imx->bitbang.setup_transfer = spi_imx_setupxfer; > @@ -1619,13 +1626,8 @@ static int spi_imx_probe(struct platform_device *pdev) > goto out_clk_put; > } > > - if (!spi_imx->slave_mode) { > - if (!master->cs_gpios) { > - dev_err(&pdev->dev, "No CS GPIOs available\n"); > - ret = -EINVAL; > - goto out_clk_put; > - } > - > + /* Request GPIO CS lines, if any */ > + if (!spi_imx->slave_mode && master->cs_gpios) { > for (i = 0; i < master->num_chipselect; i++) { > if (!gpio_is_valid(master->cs_gpios[i])) > continue; >
On 06.11.2017 20:46, Trent Piepho wrote: > If the array is not present, assume all chip selects are native. This > is the standard behavior for SPI masters configured via the device > tree and the behavior of this driver as well when it is configured via > device tree. > > This reduces platform data vs DT differences and allows most of the > platform data based boards to remove their chip select arrays. > > CC: Shawn Guo <shawnguo@kernel.org> > CC: Sascha Hauer <kernel@pengutronix.de> > CC: Fabio Estevam <fabio.estevam@nxp.com> > CC: Mark Brown <broonie@kernel.org> > Signed-off-by: Trent Piepho <tpiepho@impinj.com> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > drivers/spi/spi-imx.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index ad54f8258513..5caa6c9524fa 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -1526,13 +1526,15 @@ static int spi_imx_probe(struct platform_device *pdev) > /* Get number of chip selects, either platform data or OF */ > if (mxc_platform_info) { > master->num_chipselect = mxc_platform_info->num_chipselect; > - master->cs_gpios = devm_kzalloc(&master->dev, > - sizeof(int) * master->num_chipselect, GFP_KERNEL); > - if (!master->cs_gpios) > - return -ENOMEM; > - > - for (i = 0; i < master->num_chipselect; i++) > - master->cs_gpios[i] = mxc_platform_info->chipselect[i]; > + if (mxc_platform_info->chipselect) { > + master->cs_gpios = devm_kzalloc(&master->dev, > + sizeof(int) * master->num_chipselect, GFP_KERNEL); > + if (!master->cs_gpios) > + return -ENOMEM; > + > + for (i = 0; i < master->num_chipselect; i++) > + master->cs_gpios[i] = mxc_platform_info->chipselect[i]; > + } > } else { > u32 num_cs; > >
On Mon, Nov 06, 2017 at 11:46:16AM -0800, Trent Piepho wrote: > The driver will fail to load if the device tree property for gpio chip > selects is not specified. Change the driver to make the property optional. > Also have the driver respect the "num-cs" property in the standard way. This doesn't apply against current code, please check and resend.