[PATCH v1 1/2] dt-binding: iio: add NPCM ADC reset support

[PATCH v1 1/2] dt-binding: iio: add NPCM ADC reset support

[Tomer Maimon]

Add NPCM ADC reset binding documentation.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
index eb939fe77836..ef8eeec1a997 100644
--- a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
@@ -6,6 +6,7 @@ Required properties:
 - compatible: "nuvoton,npcm750-adc" for the NPCM7XX BMC.
 - reg: specifies physical base address and size of the registers.
 - interrupts: Contain the ADC interrupt with flags for falling edge.
+- resets : phandle to the reset control for this device.
 
 Optional properties:
 - clocks: phandle of ADC reference clock, in case the clock is not
@@ -21,4 +22,5 @@ adc: adc@f000c000 {
 	reg = <0xf000c000 0x8>;
 	interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
 	clocks = <&clk NPCM7XX_CLK_ADC>;
+	resets = <&rstc NPCM7XX_RESET_IPSRST1 NPCM7XX_RESET_ADC>;
 };
-- 
2.22.0

[PATCH v1 2/2] iio: adc: modify NPCM reset support

[Tomer Maimon]

Modify NPCM ADC reset support from
direct register access to reset controller support.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/iio/adc/npcm_adc.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
index a6170a37ebe8..83bad2d5575d 100644
--- a/drivers/iio/adc/npcm_adc.c
+++ b/drivers/iio/adc/npcm_adc.c
@@ -14,6 +14,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/spinlock.h>
 #include <linux/uaccess.h>
+#include <linux/reset.h>
 
 struct npcm_adc {
 	bool int_status;
@@ -23,13 +24,9 @@ struct npcm_adc {
 	struct clk *adc_clk;
 	wait_queue_head_t wq;
 	struct regulator *vref;
-	struct regmap *rst_regmap;
+	struct reset_control *reset;
 };
 
-/* NPCM7xx reset module */
-#define NPCM7XX_IPSRST1_OFFSET		0x020
-#define NPCM7XX_IPSRST1_ADC_RST		BIT(27)
-
 /* ADC registers */
 #define NPCM_ADCCON	 0x00
 #define NPCM_ADCDATA	 0x04
@@ -106,13 +103,11 @@ static int npcm_adc_read(struct npcm_adc *info, int *val, u8 channel)
 					       msecs_to_jiffies(10));
 	if (ret == 0) {
 		regtemp = ioread32(info->regs + NPCM_ADCCON);
-		if ((regtemp & NPCM_ADCCON_ADC_CONV) && info->rst_regmap) {
+		if (regtemp & NPCM_ADCCON_ADC_CONV) {
 			/* if conversion failed - reset ADC module */
-			regmap_write(info->rst_regmap, NPCM7XX_IPSRST1_OFFSET,
-				     NPCM7XX_IPSRST1_ADC_RST);
+			reset_control_assert(info->reset);
 			msleep(100);
-			regmap_write(info->rst_regmap, NPCM7XX_IPSRST1_OFFSET,
-				     0x0);
+			reset_control_deassert(info->reset);
 			msleep(100);
 
 			/* Enable ADC and start conversion module */
@@ -186,7 +181,6 @@ static int npcm_adc_probe(struct platform_device *pdev)
 	struct npcm_adc *info;
 	struct iio_dev *indio_dev;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = pdev->dev.of_node;
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
 	if (!indio_dev)
@@ -199,6 +193,10 @@ static int npcm_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(info->regs))
 		return PTR_ERR(info->regs);
 
+	info->reset = devm_reset_control_get(&pdev->dev, NULL);
+	if (IS_ERR(info->reset))
+		return PTR_ERR(info->reset);
+
 	info->adc_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(info->adc_clk)) {
 		dev_warn(&pdev->dev, "ADC clock failed: can't read clk\n");
@@ -211,16 +209,6 @@ static int npcm_adc_probe(struct platform_device *pdev)
 	div = div >> NPCM_ADCCON_DIV_SHIFT;
 	info->adc_sample_hz = clk_get_rate(info->adc_clk) / ((div + 1) * 2);
 
-	if (of_device_is_compatible(np, "nuvoton,npcm750-adc")) {
-		info->rst_regmap = syscon_regmap_lookup_by_compatible
-			("nuvoton,npcm750-rst");
-		if (IS_ERR(info->rst_regmap)) {
-			dev_err(&pdev->dev, "Failed to find nuvoton,npcm750-rst\n");
-			ret = PTR_ERR(info->rst_regmap);
-			goto err_disable_clk;
-		}
-	}
-
 	irq = platform_get_irq(pdev, 0);
 	if (irq <= 0) {
 		ret = -EINVAL;
-- 
2.22.0

Re: [PATCH v1 1/2] dt-binding: iio: add NPCM ADC reset support

[Rob Herring]

On Sun, 19 Jan 2020 13:00:31 +0200, Tomer Maimon wrote:
> Add NPCM ADC reset binding documentation.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v1 2/2] iio: adc: modify NPCM reset support

[Jonathan Cameron]

On Sun, 19 Jan 2020 13:00:32 +0200
Tomer Maimon <tmaimon77@gmail.com> wrote:

> Modify NPCM ADC reset support from
> direct register access to reset controller support.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>

Hmm.  This presumably breaks all old DT. 

If that's not a problem please say why.

Jonathan

> ---
>  drivers/iio/adc/npcm_adc.c | 30 +++++++++---------------------
>  1 file changed, 9 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
> index a6170a37ebe8..83bad2d5575d 100644
> --- a/drivers/iio/adc/npcm_adc.c
> +++ b/drivers/iio/adc/npcm_adc.c
> @@ -14,6 +14,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/spinlock.h>
>  #include <linux/uaccess.h>
> +#include <linux/reset.h>
>  
>  struct npcm_adc {
>  	bool int_status;
> @@ -23,13 +24,9 @@ struct npcm_adc {
>  	struct clk *adc_clk;
>  	wait_queue_head_t wq;
>  	struct regulator *vref;
> -	struct regmap *rst_regmap;
> +	struct reset_control *reset;
>  };
>  
> -/* NPCM7xx reset module */
> -#define NPCM7XX_IPSRST1_OFFSET		0x020
> -#define NPCM7XX_IPSRST1_ADC_RST		BIT(27)
> -
>  /* ADC registers */
>  #define NPCM_ADCCON	 0x00
>  #define NPCM_ADCDATA	 0x04
> @@ -106,13 +103,11 @@ static int npcm_adc_read(struct npcm_adc *info, int *val, u8 channel)
>  					       msecs_to_jiffies(10));
>  	if (ret == 0) {
>  		regtemp = ioread32(info->regs + NPCM_ADCCON);
> -		if ((regtemp & NPCM_ADCCON_ADC_CONV) && info->rst_regmap) {
> +		if (regtemp & NPCM_ADCCON_ADC_CONV) {
>  			/* if conversion failed - reset ADC module */
> -			regmap_write(info->rst_regmap, NPCM7XX_IPSRST1_OFFSET,
> -				     NPCM7XX_IPSRST1_ADC_RST);
> +			reset_control_assert(info->reset);
>  			msleep(100);
> -			regmap_write(info->rst_regmap, NPCM7XX_IPSRST1_OFFSET,
> -				     0x0);
> +			reset_control_deassert(info->reset);
>  			msleep(100);
>  
>  			/* Enable ADC and start conversion module */
> @@ -186,7 +181,6 @@ static int npcm_adc_probe(struct platform_device *pdev)
>  	struct npcm_adc *info;
>  	struct iio_dev *indio_dev;
>  	struct device *dev = &pdev->dev;
> -	struct device_node *np = pdev->dev.of_node;
>  
>  	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
>  	if (!indio_dev)
> @@ -199,6 +193,10 @@ static int npcm_adc_probe(struct platform_device *pdev)
>  	if (IS_ERR(info->regs))
>  		return PTR_ERR(info->regs);
>  
> +	info->reset = devm_reset_control_get(&pdev->dev, NULL);
> +	if (IS_ERR(info->reset))
> +		return PTR_ERR(info->reset);
> +
>  	info->adc_clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(info->adc_clk)) {
>  		dev_warn(&pdev->dev, "ADC clock failed: can't read clk\n");
> @@ -211,16 +209,6 @@ static int npcm_adc_probe(struct platform_device *pdev)
>  	div = div >> NPCM_ADCCON_DIV_SHIFT;
>  	info->adc_sample_hz = clk_get_rate(info->adc_clk) / ((div + 1) * 2);
>  
> -	if (of_device_is_compatible(np, "nuvoton,npcm750-adc")) {
> -		info->rst_regmap = syscon_regmap_lookup_by_compatible
> -			("nuvoton,npcm750-rst");
> -		if (IS_ERR(info->rst_regmap)) {
> -			dev_err(&pdev->dev, "Failed to find nuvoton,npcm750-rst\n");
> -			ret = PTR_ERR(info->rst_regmap);
> -			goto err_disable_clk;
> -		}
> -	}
> -
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq <= 0) {
>  		ret = -EINVAL;

Re: [PATCH v1 2/2] iio: adc: modify NPCM reset support

[Tomer Maimon]

[-- Attachment #1: Type: text/plain, Size: 4690 bytes --]

Hi Jonathan,

The patch replace reset ADC method from direct register access to using
reset driver (will applied next Linux version).
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next-history.git/commit/?h=next-20200130&id=9c81b2ccf82da6e995b63e945afa882cfaa03ca9


The ADC dt-binding modified as well to use the new reset method (approved
by Rob Herring)
https://www.spinics.net/lists/devicetree/msg331327.html

Indeed the this modification require DT modification as it described in the
ADC dt-binding commit, is it an issue? Do you thnk I should describe it in
the commit?

Thanks,

Tomer



On Wed, 29 Jan 2020 at 22:01, Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun, 19 Jan 2020 13:00:32 +0200
> Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> > Modify NPCM ADC reset support from
> > direct register access to reset controller support.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>
> Hmm.  This presumably breaks all old DT.
>
> If that's not a problem please say why.
>
> Jonathan
>
> > ---
> >  drivers/iio/adc/npcm_adc.c | 30 +++++++++---------------------
> >  1 file changed, 9 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
> > index a6170a37ebe8..83bad2d5575d 100644
> > --- a/drivers/iio/adc/npcm_adc.c
> > +++ b/drivers/iio/adc/npcm_adc.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/spinlock.h>
> >  #include <linux/uaccess.h>
> > +#include <linux/reset.h>
> >
> >  struct npcm_adc {
> >       bool int_status;
> > @@ -23,13 +24,9 @@ struct npcm_adc {
> >       struct clk *adc_clk;
> >       wait_queue_head_t wq;
> >       struct regulator *vref;
> > -     struct regmap *rst_regmap;
> > +     struct reset_control *reset;
> >  };
> >
> > -/* NPCM7xx reset module */
> > -#define NPCM7XX_IPSRST1_OFFSET               0x020
> > -#define NPCM7XX_IPSRST1_ADC_RST              BIT(27)
> > -
> >  /* ADC registers */
> >  #define NPCM_ADCCON   0x00
> >  #define NPCM_ADCDATA  0x04
> > @@ -106,13 +103,11 @@ static int npcm_adc_read(struct npcm_adc *info,
> int *val, u8 channel)
> >                                              msecs_to_jiffies(10));
> >       if (ret == 0) {
> >               regtemp = ioread32(info->regs + NPCM_ADCCON);
> > -             if ((regtemp & NPCM_ADCCON_ADC_CONV) && info->rst_regmap) {
> > +             if (regtemp & NPCM_ADCCON_ADC_CONV) {
> >                       /* if conversion failed - reset ADC module */
> > -                     regmap_write(info->rst_regmap,
> NPCM7XX_IPSRST1_OFFSET,
> > -                                  NPCM7XX_IPSRST1_ADC_RST);
> > +                     reset_control_assert(info->reset);
> >                       msleep(100);
> > -                     regmap_write(info->rst_regmap,
> NPCM7XX_IPSRST1_OFFSET,
> > -                                  0x0);
> > +                     reset_control_deassert(info->reset);
> >                       msleep(100);
> >
> >                       /* Enable ADC and start conversion module */
> > @@ -186,7 +181,6 @@ static int npcm_adc_probe(struct platform_device
> *pdev)
> >       struct npcm_adc *info;
> >       struct iio_dev *indio_dev;
> >       struct device *dev = &pdev->dev;
> > -     struct device_node *np = pdev->dev.of_node;
> >
> >       indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> >       if (!indio_dev)
> > @@ -199,6 +193,10 @@ static int npcm_adc_probe(struct platform_device
> *pdev)
> >       if (IS_ERR(info->regs))
> >               return PTR_ERR(info->regs);
> >
> > +     info->reset = devm_reset_control_get(&pdev->dev, NULL);
> > +     if (IS_ERR(info->reset))
> > +             return PTR_ERR(info->reset);
> > +
> >       info->adc_clk = devm_clk_get(&pdev->dev, NULL);
> >       if (IS_ERR(info->adc_clk)) {
> >               dev_warn(&pdev->dev, "ADC clock failed: can't read clk\n");
> > @@ -211,16 +209,6 @@ static int npcm_adc_probe(struct platform_device
> *pdev)
> >       div = div >> NPCM_ADCCON_DIV_SHIFT;
> >       info->adc_sample_hz = clk_get_rate(info->adc_clk) / ((div + 1) *
> 2);
> >
> > -     if (of_device_is_compatible(np, "nuvoton,npcm750-adc")) {
> > -             info->rst_regmap = syscon_regmap_lookup_by_compatible
> > -                     ("nuvoton,npcm750-rst");
> > -             if (IS_ERR(info->rst_regmap)) {
> > -                     dev_err(&pdev->dev, "Failed to find
> nuvoton,npcm750-rst\n");
> > -                     ret = PTR_ERR(info->rst_regmap);
> > -                     goto err_disable_clk;
> > -             }
> > -     }
> > -
> >       irq = platform_get_irq(pdev, 0);
> >       if (irq <= 0) {
> >               ret = -EINVAL;
>
>

[-- Attachment #2: Type: text/html, Size: 6651 bytes --]

Re: [PATCH v1 2/2] iio: adc: modify NPCM reset support

[Jonathan Cameron]

On Thu, 30 Jan 2020 10:20:30 +0200
Tomer Maimon <tmaimon77@gmail.com> wrote:

> Hi Jonathan,
> 
> The patch replace reset ADC method from direct register access to using
> reset driver (will applied next Linux version).
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next-history.git/commit/?h=next-20200130&id=9c81b2ccf82da6e995b63e945afa882cfaa03ca9
> 
> 
> The ADC dt-binding modified as well to use the new reset method (approved
> by Rob Herring)
> https://www.spinics.net/lists/devicetree/msg331327.html
> 
> Indeed the this modification require DT modification as it described in the
> ADC dt-binding commit, is it an issue? Do you thnk I should describe it in
> the commit?

Whether it is an issue depends on nuvoton business model.  Can they ensure
that all places the kernel is changed will also have appropriate dt updates?

If not, you need to make the code continue to function without the change
(fall back to old methods).  If they do have enough control to ensure it won't
break any systems out in the wild, then just add a note to say that is the
case to the commit message.

Reasons this sort of thing is sometimes safe include:
* preproduction part so no users outside of people who will expect
potential breakage
* dt and new kernel images only distributed in a firmware package, or
where build script used to build the firmware package will ensure they
match (this is a bmc chip I think, so I guess not many people build their own
kernels except via scripting to package everything up as a firmware image
of some type?)

Thanks,

Jonathan
> 
> Thanks,
> 
> Tomer
> 
> 
> 
> On Wed, 29 Jan 2020 at 22:01, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Sun, 19 Jan 2020 13:00:32 +0200
> > Tomer Maimon <tmaimon77@gmail.com> wrote:
> >  
> > > Modify NPCM ADC reset support from
> > > direct register access to reset controller support.
> > >
> > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>  
> >
> > Hmm.  This presumably breaks all old DT.
> >
> > If that's not a problem please say why.
> >
> > Jonathan
> >  
> > > ---
> > >  drivers/iio/adc/npcm_adc.c | 30 +++++++++---------------------
> > >  1 file changed, 9 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
> > > index a6170a37ebe8..83bad2d5575d 100644
> > > --- a/drivers/iio/adc/npcm_adc.c
> > > +++ b/drivers/iio/adc/npcm_adc.c
> > > @@ -14,6 +14,7 @@
> > >  #include <linux/regulator/consumer.h>
> > >  #include <linux/spinlock.h>
> > >  #include <linux/uaccess.h>
> > > +#include <linux/reset.h>
> > >
> > >  struct npcm_adc {
> > >       bool int_status;
> > > @@ -23,13 +24,9 @@ struct npcm_adc {
> > >       struct clk *adc_clk;
> > >       wait_queue_head_t wq;
> > >       struct regulator *vref;
> > > -     struct regmap *rst_regmap;
> > > +     struct reset_control *reset;
> > >  };
> > >
> > > -/* NPCM7xx reset module */
> > > -#define NPCM7XX_IPSRST1_OFFSET               0x020
> > > -#define NPCM7XX_IPSRST1_ADC_RST              BIT(27)
> > > -
> > >  /* ADC registers */
> > >  #define NPCM_ADCCON   0x00
> > >  #define NPCM_ADCDATA  0x04
> > > @@ -106,13 +103,11 @@ static int npcm_adc_read(struct npcm_adc *info,  
> > int *val, u8 channel)  
> > >                                              msecs_to_jiffies(10));
> > >       if (ret == 0) {
> > >               regtemp = ioread32(info->regs + NPCM_ADCCON);
> > > -             if ((regtemp & NPCM_ADCCON_ADC_CONV) && info->rst_regmap) {
> > > +             if (regtemp & NPCM_ADCCON_ADC_CONV) {
> > >                       /* if conversion failed - reset ADC module */
> > > -                     regmap_write(info->rst_regmap,  
> > NPCM7XX_IPSRST1_OFFSET,  
> > > -                                  NPCM7XX_IPSRST1_ADC_RST);
> > > +                     reset_control_assert(info->reset);
> > >                       msleep(100);
> > > -                     regmap_write(info->rst_regmap,  
> > NPCM7XX_IPSRST1_OFFSET,  
> > > -                                  0x0);
> > > +                     reset_control_deassert(info->reset);
> > >                       msleep(100);
> > >
> > >                       /* Enable ADC and start conversion module */
> > > @@ -186,7 +181,6 @@ static int npcm_adc_probe(struct platform_device  
> > *pdev)  
> > >       struct npcm_adc *info;
> > >       struct iio_dev *indio_dev;
> > >       struct device *dev = &pdev->dev;
> > > -     struct device_node *np = pdev->dev.of_node;
> > >
> > >       indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> > >       if (!indio_dev)
> > > @@ -199,6 +193,10 @@ static int npcm_adc_probe(struct platform_device  
> > *pdev)  
> > >       if (IS_ERR(info->regs))
> > >               return PTR_ERR(info->regs);
> > >
> > > +     info->reset = devm_reset_control_get(&pdev->dev, NULL);
> > > +     if (IS_ERR(info->reset))
> > > +             return PTR_ERR(info->reset);
> > > +
> > >       info->adc_clk = devm_clk_get(&pdev->dev, NULL);
> > >       if (IS_ERR(info->adc_clk)) {
> > >               dev_warn(&pdev->dev, "ADC clock failed: can't read clk\n");
> > > @@ -211,16 +209,6 @@ static int npcm_adc_probe(struct platform_device  
> > *pdev)  
> > >       div = div >> NPCM_ADCCON_DIV_SHIFT;
> > >       info->adc_sample_hz = clk_get_rate(info->adc_clk) / ((div + 1) *  
> > 2);  
> > >
> > > -     if (of_device_is_compatible(np, "nuvoton,npcm750-adc")) {
> > > -             info->rst_regmap = syscon_regmap_lookup_by_compatible
> > > -                     ("nuvoton,npcm750-rst");
> > > -             if (IS_ERR(info->rst_regmap)) {
> > > -                     dev_err(&pdev->dev, "Failed to find  
> > nuvoton,npcm750-rst\n");  
> > > -                     ret = PTR_ERR(info->rst_regmap);
> > > -                     goto err_disable_clk;
> > > -             }
> > > -     }
> > > -
> > >       irq = platform_get_irq(pdev, 0);
> > >       if (irq <= 0) {
> > >               ret = -EINVAL;  
> >
> >  

Re: [PATCH v1 2/2] iio: adc: modify NPCM reset support

[Tomer Maimon]

[-- Attachment #1: Type: text/plain, Size: 6587 bytes --]

Hi Jonathan,

Thanks for the clarification,


On Sun, 2 Feb 2020 at 13:28, Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 30 Jan 2020 10:20:30 +0200
> Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> > Hi Jonathan,
> >
> > The patch replace reset ADC method from direct register access to using
> > reset driver (will applied next Linux version).
> >
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next-history.git/commit/?h=next-20200130&id=9c81b2ccf82da6e995b63e945afa882cfaa03ca9
> >
> >
> > The ADC dt-binding modified as well to use the new reset method (approved
> > by Rob Herring)
> > https://www.spinics.net/lists/devicetree/msg331327.html
> >
> > Indeed the this modification require DT modification as it described in
> the
> > ADC dt-binding commit, is it an issue? Do you thnk I should describe it
> in
> > the commit?
>
> Whether it is an issue depends on nuvoton business model.  Can they ensure
> that all places the kernel is changed will also have appropriate dt
> updates?

Yes,  we can ensure it in this development stage.


>
If not, you need to make the code continue to function without the change
> (fall back to old methods).  If they do have enough control to ensure it
> won't
> break any systems out in the wild, then just add a note to say that is the
> case to the commit message.
>
I will modify the commit message as you suggested.

Reasons this sort of thing is sometimes safe include:
> * preproduction part so no users outside of people who will expect
> potential breakage
> * dt and new kernel images only distributed in a firmware package, or
> where build script used to build the firmware package will ensure they
> match (this is a bmc chip I think, so I guess not many people build their
> own
> kernels except via scripting to package everything up as a firmware image
> of some type?)
>
As far as I know only the server developers are creating new firmwares and
they are aware to our modifications.

>
> Thanks,
>
> Jonathan

>
> > Thanks,
> >
> > Tomer
> >
> >
> >
> > On Wed, 29 Jan 2020 at 22:01, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > > On Sun, 19 Jan 2020 13:00:32 +0200
> > > Tomer Maimon <tmaimon77@gmail.com> wrote:
> > >
> > > > Modify NPCM ADC reset support from
> > > > direct register access to reset controller support.
> > > >
> > > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > >
> > > Hmm.  This presumably breaks all old DT.
> > >
> > > If that's not a problem please say why.
> > >
> > > Jonathan
> > >
> > > > ---
> > > >  drivers/iio/adc/npcm_adc.c | 30 +++++++++---------------------
> > > >  1 file changed, 9 insertions(+), 21 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
> > > > index a6170a37ebe8..83bad2d5575d 100644
> > > > --- a/drivers/iio/adc/npcm_adc.c
> > > > +++ b/drivers/iio/adc/npcm_adc.c
> > > > @@ -14,6 +14,7 @@
> > > >  #include <linux/regulator/consumer.h>
> > > >  #include <linux/spinlock.h>
> > > >  #include <linux/uaccess.h>
> > > > +#include <linux/reset.h>
> > > >
> > > >  struct npcm_adc {
> > > >       bool int_status;
> > > > @@ -23,13 +24,9 @@ struct npcm_adc {
> > > >       struct clk *adc_clk;
> > > >       wait_queue_head_t wq;
> > > >       struct regulator *vref;
> > > > -     struct regmap *rst_regmap;
> > > > +     struct reset_control *reset;
> > > >  };
> > > >
> > > > -/* NPCM7xx reset module */
> > > > -#define NPCM7XX_IPSRST1_OFFSET               0x020
> > > > -#define NPCM7XX_IPSRST1_ADC_RST              BIT(27)
> > > > -
> > > >  /* ADC registers */
> > > >  #define NPCM_ADCCON   0x00
> > > >  #define NPCM_ADCDATA  0x04
> > > > @@ -106,13 +103,11 @@ static int npcm_adc_read(struct npcm_adc
> *info,
> > > int *val, u8 channel)
> > > >                                              msecs_to_jiffies(10));
> > > >       if (ret == 0) {
> > > >               regtemp = ioread32(info->regs + NPCM_ADCCON);
> > > > -             if ((regtemp & NPCM_ADCCON_ADC_CONV) &&
> info->rst_regmap) {
> > > > +             if (regtemp & NPCM_ADCCON_ADC_CONV) {
> > > >                       /* if conversion failed - reset ADC module */
> > > > -                     regmap_write(info->rst_regmap,
> > > NPCM7XX_IPSRST1_OFFSET,
> > > > -                                  NPCM7XX_IPSRST1_ADC_RST);
> > > > +                     reset_control_assert(info->reset);
> > > >                       msleep(100);
> > > > -                     regmap_write(info->rst_regmap,
> > > NPCM7XX_IPSRST1_OFFSET,
> > > > -                                  0x0);
> > > > +                     reset_control_deassert(info->reset);
> > > >                       msleep(100);
> > > >
> > > >                       /* Enable ADC and start conversion module */
> > > > @@ -186,7 +181,6 @@ static int npcm_adc_probe(struct
> platform_device
> > > *pdev)
> > > >       struct npcm_adc *info;
> > > >       struct iio_dev *indio_dev;
> > > >       struct device *dev = &pdev->dev;
> > > > -     struct device_node *np = pdev->dev.of_node;
> > > >
> > > >       indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> > > >       if (!indio_dev)
> > > > @@ -199,6 +193,10 @@ static int npcm_adc_probe(struct
> platform_device
> > > *pdev)
> > > >       if (IS_ERR(info->regs))
> > > >               return PTR_ERR(info->regs);
> > > >
> > > > +     info->reset = devm_reset_control_get(&pdev->dev, NULL);
> > > > +     if (IS_ERR(info->reset))
> > > > +             return PTR_ERR(info->reset);
> > > > +
> > > >       info->adc_clk = devm_clk_get(&pdev->dev, NULL);
> > > >       if (IS_ERR(info->adc_clk)) {
> > > >               dev_warn(&pdev->dev, "ADC clock failed: can't read
> clk\n");
> > > > @@ -211,16 +209,6 @@ static int npcm_adc_probe(struct
> platform_device
> > > *pdev)
> > > >       div = div >> NPCM_ADCCON_DIV_SHIFT;
> > > >       info->adc_sample_hz = clk_get_rate(info->adc_clk) / ((div + 1)
> *
> > > 2);
> > > >
> > > > -     if (of_device_is_compatible(np, "nuvoton,npcm750-adc")) {
> > > > -             info->rst_regmap = syscon_regmap_lookup_by_compatible
> > > > -                     ("nuvoton,npcm750-rst");
> > > > -             if (IS_ERR(info->rst_regmap)) {
> > > > -                     dev_err(&pdev->dev, "Failed to find
> > > nuvoton,npcm750-rst\n");
> > > > -                     ret = PTR_ERR(info->rst_regmap);
> > > > -                     goto err_disable_clk;
> > > > -             }
> > > > -     }
> > > > -
> > > >       irq = platform_get_irq(pdev, 0);
> > > >       if (irq <= 0) {
> > > >               ret = -EINVAL;
> > >
> > >
>
>
Thanks a lot,

Tomer

[-- Attachment #2: Type: text/html, Size: 10333 bytes --]