[PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator

Peng Fan (OSS) posted 3 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator
Posted by Peng Fan (OSS) 1 month, 4 weeks ago
From: Peng Fan <peng.fan@nxp.com>

SPI NOR flashes needs power supply to work properly. The power supply
maybe software controllable per board design. So add the support
for an vcc-supply regulator.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mtd/spi-nor/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 9d6e85bf227b..5249c8b13916 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -17,6 +17,7 @@
 #include <linux/mtd/spi-nor.h>
 #include <linux/mutex.h>
 #include <linux/of_platform.h>
+#include <linux/regulator/consumer.h>
 #include <linux/sched/task_stack.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
@@ -3462,6 +3463,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (!nor->bouncebuf)
 		return -ENOMEM;
 
+	ret = devm_regulator_get_enable(dev, "vcc");
+	if (ret)
+		return ret;
+
 	ret = spi_nor_hw_reset(nor);
 	if (ret)
 		return ret;

-- 
2.37.1
Re: [PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator
Posted by Marc Kleine-Budde 1 month, 4 weeks ago
On 30.09.2024 17:22:25, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> SPI NOR flashes needs power supply to work properly. The power supply
> maybe software controllable per board design. So add the support
> for an vcc-supply regulator.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/mtd/spi-nor/core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 9d6e85bf227b..5249c8b13916 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -17,6 +17,7 @@
>  #include <linux/mtd/spi-nor.h>
>  #include <linux/mutex.h>
>  #include <linux/of_platform.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> @@ -3462,6 +3463,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	if (!nor->bouncebuf)
>  		return -ENOMEM;
>  
> +	ret = devm_regulator_get_enable(dev, "vcc");
> +	if (ret)
> +		return ret;
> +

What happens if the SPI-NOR doesn't have a "vcc" regulator?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator
Posted by Marc Kleine-Budde 1 month, 4 weeks ago
On 30.09.2024 11:21:27, Marc Kleine-Budde wrote:
> On 30.09.2024 17:22:25, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> > 
> > SPI NOR flashes needs power supply to work properly. The power supply
> > maybe software controllable per board design. So add the support
> > for an vcc-supply regulator.
> > 
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/mtd/spi-nor/core.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 9d6e85bf227b..5249c8b13916 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/mtd/spi-nor.h>
> >  #include <linux/mutex.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/regulator/consumer.h>
> >  #include <linux/sched/task_stack.h>
> >  #include <linux/sizes.h>
> >  #include <linux/slab.h>
> > @@ -3462,6 +3463,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> >  	if (!nor->bouncebuf)
> >  		return -ENOMEM;
> >  
> > +	ret = devm_regulator_get_enable(dev, "vcc");
> > +	if (ret)
> > +		return ret;
> > +
> 
> What happens if the SPI-NOR doesn't have a "vcc" regulator?

...the SPI-NOR will use the dummy regulator.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator
Posted by Michael Walle 1 month, 3 weeks ago
Hi,

> > > +	ret = devm_regulator_get_enable(dev, "vcc");
> > > +	if (ret)
> > > +		return ret;
> > > +
> > 
> > What happens if the SPI-NOR doesn't have a "vcc" regulator?
>
> ...the SPI-NOR will use the dummy regulator.

Which then prints a warning "using dummy regulator". Is that the
usual way to go?
I mean the regulator is actually mandatory because it is the main
voltage rail for the flash. To get rid of the warning one can add a
fixed-regulator (which is correct anyway). But OTOH, the device tree
lists it as optional (marking it as required isn't an option either
because virtually all device trees won't have that property).

-michael
Re: [PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator
Posted by Tudor Ambarus 1 month ago

On 10/7/24 8:50 AM, Michael Walle wrote:
> Hi,
> 
>>>> +	ret = devm_regulator_get_enable(dev, "vcc");
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>
>>> What happens if the SPI-NOR doesn't have a "vcc" regulator?
>>
>> ...the SPI-NOR will use the dummy regulator.
> 
> Which then prints a warning "using dummy regulator". Is that the
> usual way to go?
> I mean the regulator is actually mandatory because it is the main
> voltage rail for the flash. To get rid of the warning one can add a
> fixed-regulator (which is correct anyway). But OTOH, the device tree

I think it's fine to have the warning. It will help people write a
better device tree.

> lists it as optional (marking it as required isn't an option either
> because virtually all device trees won't have that property).
> 
> -michael
Re: [PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator
Posted by Miquel Raynal 1 month, 4 weeks ago
Hi,

peng.fan@oss.nxp.com wrote on Mon, 30 Sep 2024 17:22:25 +0800:

> From: Peng Fan <peng.fan@nxp.com>
> 
> SPI NOR flashes needs power supply to work properly. The power supply
> maybe software controllable per board design. So add the support
> for an vcc-supply regulator.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/mtd/spi-nor/core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 9d6e85bf227b..5249c8b13916 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -17,6 +17,7 @@
>  #include <linux/mtd/spi-nor.h>
>  #include <linux/mutex.h>
>  #include <linux/of_platform.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> @@ -3462,6 +3463,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	if (!nor->bouncebuf)
>  		return -ENOMEM;
>  
> +	ret = devm_regulator_get_enable(dev, "vcc");

					_optional ?

> +	if (ret)
> +		return ret;
> +
>  	ret = spi_nor_hw_reset(nor);
>  	if (ret)
>  		return ret;
> 


Thanks,
Miquèl
Re: [PATCH v2 2/3] mtd: spi-nor: support vcc-supply regulator
Posted by Marco Felsch 1 month, 4 weeks ago
Hi Miquel,

On 24-09-30, Miquel Raynal wrote:
> Hi,
> 
> peng.fan@oss.nxp.com wrote on Mon, 30 Sep 2024 17:22:25 +0800:
> 
> > From: Peng Fan <peng.fan@nxp.com>
> > 
> > SPI NOR flashes needs power supply to work properly. The power supply
> > maybe software controllable per board design. So add the support
> > for an vcc-supply regulator.
> > 
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/mtd/spi-nor/core.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 9d6e85bf227b..5249c8b13916 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/mtd/spi-nor.h>
> >  #include <linux/mutex.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/regulator/consumer.h>
> >  #include <linux/sched/task_stack.h>
> >  #include <linux/sizes.h>
> >  #include <linux/slab.h>
> > @@ -3462,6 +3463,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> >  	if (!nor->bouncebuf)
> >  		return -ENOMEM;
> >  
> > +	ret = devm_regulator_get_enable(dev, "vcc");
> 
> 					_optional ?

The regulator optional API is different compared to other optional APIs.
If we would use optional here, we would need to check the returned error
code. On the other hand if the non optional API is used and the
regualtor is missing, a dummy regualtor is returned (as pointed out by
Marc).

Please see the _optional API doc to see more information about the
_optional usage.

Regards,
  Marco

> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = spi_nor_hw_reset(nor);
> >  	if (ret)
> >  		return ret;
> > 
> 
> 
> Thanks,
> Miquèl
>