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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7d79c45a37f64a41947f05b6cae96bd6@sphcmbx02.sunplus.com.tw>
Date:   Fri, 14 Jan 2022 01:31:41 +0000
From:   Lh Kuo 郭力豪 <lh.Kuo@...plus.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Li-hao Kuo <lhjeff911@...il.com>
CC:     Philipp Zabel <p.zabel@...gutronix.de>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-spi <linux-spi@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Wells Lu 呂芳騰 <wells.lu@...plus.com>
Subject: RE: [PATCH v5 1/2] SPI: Add SPI driver for Sunplus SP7021

> > +               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