diff mbox series

[v2,33/46] mfd: ti_am335x_tscadc: Move control register configuration

Message ID 20210902215144.507243-34-miquel.raynal@bootlin.com
State New
Headers show
Series TI AM437X ADC1 | expand

Commit Message

Miquel Raynal Sept. 2, 2021, 9:51 p.m. UTC
The datasheet states that most of the configuration should be set in the
control register in the first place, before actually enabling the
hardware. So far only half of the configuration was made in the first
step, which does not make really sense and would complicating the code
when introducing support for the am437x hardware.

Let's move that register write a bit below to enclose more configuration.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Grygorii Strashko Sept. 3, 2021, 1:24 p.m. UTC | #1
On 03/09/2021 00:51, Miquel Raynal wrote:
> The datasheet states that most of the configuration should be set in the
> control register in the first place, before actually enabling the
> hardware. So far only half of the configuration was made in the first
> step, which does not make really sense and would complicating the code
> when introducing support for the am437x hardware.
> 
> Let's move that register write a bit below to enclose more configuration.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>   drivers/mfd/ti_am335x_tscadc.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 29ada9da8826..a0db3e4ff265 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -239,6 +239,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>   	}
>   	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
>   
> +	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
> +

Strange change - above 2 lines a the same !?

>   	tscadc_idle_config(tscadc);
>   
>   	/* Enable the TSC module enable bit */
>
Jonathan Cameron Sept. 5, 2021, 1:18 p.m. UTC | #2
On Fri, 3 Sep 2021 16:24:22 +0300
Grygorii Strashko <grygorii.strashko@ti.com> wrote:

> On 03/09/2021 00:51, Miquel Raynal wrote:
> > The datasheet states that most of the configuration should be set in the
> > control register in the first place, before actually enabling the
> > hardware. So far only half of the configuration was made in the first
> > step, which does not make really sense and would complicating the code
> > when introducing support for the am437x hardware.
> > 
> > Let's move that register write a bit below to enclose more configuration.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >   drivers/mfd/ti_am335x_tscadc.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> > index 29ada9da8826..a0db3e4ff265 100644
> > --- a/drivers/mfd/ti_am335x_tscadc.c
> > +++ b/drivers/mfd/ti_am335x_tscadc.c
> > @@ -239,6 +239,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
> >   	}
> >   	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
> >   
> > +	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
> > +  
> 
> Strange change - above 2 lines a the same !?

Yup, Looks like this one got handled earlier and the rebase went wrong.
So drop it.

> 
> >   	tscadc_idle_config(tscadc);
> >   
> >   	/* Enable the TSC module enable bit */
> >   
>
Miquel Raynal Sept. 5, 2021, 1:22 p.m. UTC | #3
Hi Jonathan,

jic23@kernel.org wrote on Sun, 5 Sep 2021 14:18:52 +0100:

> On Fri, 3 Sep 2021 16:24:22 +0300
> Grygorii Strashko <grygorii.strashko@ti.com> wrote:
> 
> > On 03/09/2021 00:51, Miquel Raynal wrote:  
> > > The datasheet states that most of the configuration should be set in the
> > > control register in the first place, before actually enabling the
> > > hardware. So far only half of the configuration was made in the first
> > > step, which does not make really sense and would complicating the code
> > > when introducing support for the am437x hardware.
> > > 
> > > Let's move that register write a bit below to enclose more configuration.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >   drivers/mfd/ti_am335x_tscadc.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> > > index 29ada9da8826..a0db3e4ff265 100644
> > > --- a/drivers/mfd/ti_am335x_tscadc.c
> > > +++ b/drivers/mfd/ti_am335x_tscadc.c
> > > @@ -239,6 +239,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
> > >   	}
> > >   	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
> > >   
> > > +	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
> > > +    
> > 
> > Strange change - above 2 lines a the same !?  
> 
> Yup, Looks like this one got handled earlier and the rebase went wrong.
> So drop it.

Exactly. That's a rebase issue I didn't notice.

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 29ada9da8826..a0db3e4ff265 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -239,6 +239,8 @@  static	int ti_tscadc_probe(struct platform_device *pdev)
 	}
 	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
 
+	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
+
 	tscadc_idle_config(tscadc);
 
 	/* Enable the TSC module enable bit */