On Wed, Feb 13, 2019 at 6:46 AM André Przywara <andre.przyw...@arm.com> wrote:
>
> On 09/02/2019 13:14, Jagan Teki wrote:
> > Update the existing register writes using setbits_le32 and
> > clrbits_le32 in required places.
> >
> > Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
> > ---
> >  drivers/spi/sun4i_spi.c | 21 ++++++++-------------
> >  1 file changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
> > index 87b396a96e..5446cebe7c 100644
> > --- a/drivers/spi/sun4i_spi.c
> > +++ b/drivers/spi/sun4i_spi.c
> > @@ -283,20 +283,18 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
> >  {
> >       struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
> >
> > -     writel(SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER | SUN4I_CTL_TP |
> > -            SUN4I_CTL_CS_MANUAL | SUN4I_CTL_CS_ACTIVE_LOW,
> > -            &priv->regs->ctl);
> > +     setbits_le32(&priv->regs->ctl, SUN4I_CTL_ENABLE |
> > +                  SUN4I_CTL_MASTER | SUN4I_CTL_TP |
> > +                  SUN4I_CTL_CS_MANUAL | SUN4I_CTL_CS_ACTIVE_LOW);
> > +
>
> Careful, this changes semantics. The original call explicitly cleared
> all the remaining bits, which is important for the setup (some bits are
> default 1).

Usually claiming would need to setup require bits, on that case
setbits is safer rather than complete writel

> And besides I am not sure what this change would improve anyway, as it
> isn't shorter. If at all, I'd use clrsetbits_le32(), to mask off the
> reserved bits 31:21.

It's not about the shorter and just enable require bits which is
usually do any claim call in SPI.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to