[U-Boot] [PATCH] spi: soft_spi: Fix null ptr when probing soft_spi.

[U-Boot] [PATCH] spi: soft_spi: Fix null ptr when probing soft_spi.

[Horatiu Vultur]

When probing soft_spi the result of dev_get_parent_priv(dev) in probe
function is null ptr because the spi is on the ahb bus which has
per_child_auto_alloc_size set to 0. Therefore it would generate an Ooops
messages when accessing spi_slave structure.

The fix consist of delaying the read of dm_spi_slave_platdata until a
child under the spi is probed, to be able to read SPI mode. Therefore
implement .child_pre_probe in which updates soft_spi_platdata based on
child dm_spi_slave_platdata.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/spi/soft_spi.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/soft_spi.c b/drivers/spi/soft_spi.c
index b06883f..e28591b 100644
--- a/drivers/spi/soft_spi.c
+++ b/drivers/spi/soft_spi.c
@@ -210,18 +210,13 @@ static int soft_spi_ofdata_to_platdata(struct udevice *dev)
 
 static int soft_spi_probe(struct udevice *dev)
 {
-	struct spi_slave *slave = dev_get_parent_priv(dev);
 	struct soft_spi_platdata *plat = dev->platdata;
-	int cs_flags, clk_flags;
 	int ret;
 
-	cs_flags = (slave->mode & SPI_CS_HIGH) ? 0 : GPIOD_ACTIVE_LOW;
-	clk_flags = (slave->mode & SPI_CPOL) ? GPIOD_ACTIVE_LOW : 0;
-
 	if (gpio_request_by_name(dev, "cs-gpios", 0, &plat->cs,
-				 GPIOD_IS_OUT | cs_flags) ||
+				 GPIOD_IS_OUT) ||
 	    gpio_request_by_name(dev, "gpio-sck", 0, &plat->sclk,
-				 GPIOD_IS_OUT | clk_flags))
+				 GPIOD_IS_OUT))
 		return -EINVAL;
 
 	ret = gpio_request_by_name(dev, "gpio-mosi", 0, &plat->mosi,
@@ -241,6 +236,20 @@ static int soft_spi_probe(struct udevice *dev)
 	return 0;
 }
 
+static int soft_spi_child_pre_probe(struct udevice *dev)
+{
+	struct udevice *bus = dev_get_parent(dev);
+	struct dm_spi_slave_platdata *slave = dev_get_parent_platdata(dev);
+	struct soft_spi_platdata *plat = bus->platdata;
+
+	if (!(slave->mode & SPI_CS_HIGH))
+		plat->cs.flags |= GPIOD_ACTIVE_LOW;
+	if (slave->mode & SPI_CPOL)
+		plat->sclk.flags |= GPIOD_ACTIVE_LOW;
+
+	return 0;
+}
+
 static const struct udevice_id soft_spi_ids[] = {
 	{ .compatible = "spi-gpio" },
 	{ }
@@ -254,5 +263,6 @@ U_BOOT_DRIVER(soft_spi) = {
 	.ofdata_to_platdata = soft_spi_ofdata_to_platdata,
 	.platdata_auto_alloc_size = sizeof(struct soft_spi_platdata),
 	.priv_auto_alloc_size = sizeof(struct soft_spi_priv),
+	.child_pre_probe = soft_spi_child_pre_probe,
 	.probe	= soft_spi_probe,
 };
-- 
2.7.4

[U-Boot] [PATCH] spi: soft_spi: Fix null ptr when probing soft_spi.

[Jagan Teki]

On Wed, Dec 19, 2018 at 8:40 PM Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> When probing soft_spi the result of dev_get_parent_priv(dev) in probe
> function is null ptr because the spi is on the ahb bus which has
> per_child_auto_alloc_size set to 0. Therefore it would generate an Ooops
> messages when accessing spi_slave structure.
>
> The fix consist of delaying the read of dm_spi_slave_platdata until a
> child under the spi is probed, to be able to read SPI mode. Therefore
> implement .child_pre_probe in which updates soft_spi_platdata based on
> child dm_spi_slave_platdata.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  drivers/spi/soft_spi.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/soft_spi.c b/drivers/spi/soft_spi.c
> index b06883f..e28591b 100644
> --- a/drivers/spi/soft_spi.c
> +++ b/drivers/spi/soft_spi.c
> @@ -210,18 +210,13 @@ static int soft_spi_ofdata_to_platdata(struct udevice *dev)
>
>  static int soft_spi_probe(struct udevice *dev)
>  {
> -       struct spi_slave *slave = dev_get_parent_priv(dev);
>         struct soft_spi_platdata *plat = dev->platdata;
> -       int cs_flags, clk_flags;
>         int ret;
>
> -       cs_flags = (slave->mode & SPI_CS_HIGH) ? 0 : GPIOD_ACTIVE_LOW;
> -       clk_flags = (slave->mode & SPI_CPOL) ? GPIOD_ACTIVE_LOW : 0;
> -
>         if (gpio_request_by_name(dev, "cs-gpios", 0, &plat->cs,
> -                                GPIOD_IS_OUT | cs_flags) ||
> +                                GPIOD_IS_OUT) ||
>             gpio_request_by_name(dev, "gpio-sck", 0, &plat->sclk,
> -                                GPIOD_IS_OUT | clk_flags))
> +                                GPIOD_IS_OUT))
>                 return -EINVAL;
>
>         ret = gpio_request_by_name(dev, "gpio-mosi", 0, &plat->mosi,
> @@ -241,6 +236,20 @@ static int soft_spi_probe(struct udevice *dev)
>         return 0;
>  }
>
> +static int soft_spi_child_pre_probe(struct udevice *dev)
> +{
> +       struct udevice *bus = dev_get_parent(dev);
> +       struct dm_spi_slave_platdata *slave = dev_get_parent_platdata(dev);
> +       struct soft_spi_platdata *plat = bus->platdata;
> +
> +       if (!(slave->mode & SPI_CS_HIGH))
> +               plat->cs.flags |= GPIOD_ACTIVE_LOW;
> +       if (slave->mode & SPI_CPOL)
> +               plat->sclk.flags |= GPIOD_ACTIVE_LOW;

Can't we update the flags during .set_mode? it set_mode would trigger
during initial probe time and modes were updated as part of that.

[U-Boot] [PATCH] spi: soft_spi: Fix null ptr when probing soft_spi.

[Gregory CLEMENT]

Hi Horatiu,

I am adding Daniel because actually this patch is needed to be able to
boot the Luton MIPS based platform. It is on this platform that we find
this issue.

Gregory
 
 On mer., déc. 19 2018, Horatiu Vultur <horatiu.vultur@microchip.com> wrote:

> When probing soft_spi the result of dev_get_parent_priv(dev) in probe
> function is null ptr because the spi is on the ahb bus which has
> per_child_auto_alloc_size set to 0. Therefore it would generate an Ooops
> messages when accessing spi_slave structure.
>
> The fix consist of delaying the read of dm_spi_slave_platdata until a
> child under the spi is probed, to be able to read SPI mode. Therefore
> implement .child_pre_probe in which updates soft_spi_platdata based on
> child dm_spi_slave_platdata.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  drivers/spi/soft_spi.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/soft_spi.c b/drivers/spi/soft_spi.c
> index b06883f..e28591b 100644
> --- a/drivers/spi/soft_spi.c
> +++ b/drivers/spi/soft_spi.c
> @@ -210,18 +210,13 @@ static int soft_spi_ofdata_to_platdata(struct udevice *dev)
>  
>  static int soft_spi_probe(struct udevice *dev)
>  {
> -	struct spi_slave *slave = dev_get_parent_priv(dev);
>  	struct soft_spi_platdata *plat = dev->platdata;
> -	int cs_flags, clk_flags;
>  	int ret;
>  
> -	cs_flags = (slave->mode & SPI_CS_HIGH) ? 0 : GPIOD_ACTIVE_LOW;
> -	clk_flags = (slave->mode & SPI_CPOL) ? GPIOD_ACTIVE_LOW : 0;
> -
>  	if (gpio_request_by_name(dev, "cs-gpios", 0, &plat->cs,
> -				 GPIOD_IS_OUT | cs_flags) ||
> +				 GPIOD_IS_OUT) ||
>  	    gpio_request_by_name(dev, "gpio-sck", 0, &plat->sclk,
> -				 GPIOD_IS_OUT | clk_flags))
> +				 GPIOD_IS_OUT))
>  		return -EINVAL;
>  
>  	ret = gpio_request_by_name(dev, "gpio-mosi", 0, &plat->mosi,
> @@ -241,6 +236,20 @@ static int soft_spi_probe(struct udevice *dev)
>  	return 0;
>  }
>  
> +static int soft_spi_child_pre_probe(struct udevice *dev)
> +{
> +	struct udevice *bus = dev_get_parent(dev);
> +	struct dm_spi_slave_platdata *slave = dev_get_parent_platdata(dev);
> +	struct soft_spi_platdata *plat = bus->platdata;
> +
> +	if (!(slave->mode & SPI_CS_HIGH))
> +		plat->cs.flags |= GPIOD_ACTIVE_LOW;
> +	if (slave->mode & SPI_CPOL)
> +		plat->sclk.flags |= GPIOD_ACTIVE_LOW;
> +
> +	return 0;
> +}
> +
>  static const struct udevice_id soft_spi_ids[] = {
>  	{ .compatible = "spi-gpio" },
>  	{ }
> @@ -254,5 +263,6 @@ U_BOOT_DRIVER(soft_spi) = {
>  	.ofdata_to_platdata = soft_spi_ofdata_to_platdata,
>  	.platdata_auto_alloc_size = sizeof(struct soft_spi_platdata),
>  	.priv_auto_alloc_size = sizeof(struct soft_spi_priv),
> +	.child_pre_probe = soft_spi_child_pre_probe,
>  	.probe	= soft_spi_probe,
>  };
> -- 
> 2.7.4
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

[U-Boot] [PATCH] spi: soft_spi: Fix null ptr when probing soft_spi.

[Daniel Schwierzeck]

Am Mi., 19. Dez. 2018 um 15:10 Uhr schrieb Horatiu Vultur
<horatiu.vultur@microchip.com>:
>
> When probing soft_spi the result of dev_get_parent_priv(dev) in probe
> function is null ptr because the spi is on the ahb bus which has
> per_child_auto_alloc_size set to 0. Therefore it would generate an Ooops
> messages when accessing spi_slave structure.
>
> The fix consist of delaying the read of dm_spi_slave_platdata until a
> child under the spi is probed, to be able to read SPI mode. Therefore
> implement .child_pre_probe in which updates soft_spi_platdata based on
> child dm_spi_slave_platdata.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  drivers/spi/soft_spi.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>

Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>