RE: [PATCH v5 1/2] SPI: Add SPI driver for Sunplus SP7021

[Lh Kuo 郭力豪 <lh.Kuo@xxxxxxxxxxx>]

> > +               writel(readl(pspim->m_base + SP7021_INT_BUSY_REG)
> > +                       | SP7021_CLR_MAS_INT, pspim->m_base +
> > + SP7021_INT_BUSY_REG);
> 
> It's better to read with temporary variable being used:
> 
>              value = readl(pspim->m_base + SP7021_INT_BUSY_REG);
>              value |= SP7021_CLR_MAS_INT;
>              writel(value, pspim->m_base + SP7021_INT_BUSY_REG);

I will add it in next next submission

> > +       pspim->xfer_conf |= ((clk_sel & 0xffff) << 16);
> 
> Is xfer_conf bigger than 32-bit? If not, why do you need the ' & 0xffff' part?
> 
> ...

I will add it in next next submission

> > +               mode = SP7021_SLAVE_MODE;
> 
> ...belongs to this condition, so do not interleave them.
> 
> On top of that you may use device property API:
> 
>     if (device_property_read_bool(&pdev->dev, "spi-slave"))
>         mode = SP7021_SLAVE_MODE;
>     else
>         mode = SP7021_MASTER_MODE;


I will add it in next next submission


> > +               return dev_err_probe(dev, ret, "spi_register_master fail\n");
> > +       }
> > +
> > +       return ret;
> 
> return 0;
> 
> ...
> 
> > +MODULE_LICENSE("GPL v2");
> 
> "GPL", the one you used is legacy.
>


I will add it in next next submission