[PATCH v1 2/2] iio: adc: modify NPCM reset support
Tomer Maimon
tmaimon77 at gmail.com
Mon Feb 3 19:58:06 AEDT 2020
Hi Jonathan,
Thanks for the clarification,
On Sun, 2 Feb 2020 at 13:28, Jonathan Cameron <jic23 at kernel.org> wrote:
> On Thu, 30 Jan 2020 10:20:30 +0200
> Tomer Maimon <tmaimon77 at 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 at kernel.org> wrote:
> >
> > > On Sun, 19 Jan 2020 13:00:32 +0200
> > > Tomer Maimon <tmaimon77 at gmail.com> wrote:
> > >
> > > > Modify NPCM ADC reset support from
> > > > direct register access to reset controller support.
> > > >
> > > > Signed-off-by: Tomer Maimon <tmaimon77 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20200203/252ff70d/attachment-0001.htm>
More information about the openbmc
mailing list