linux-kernel - Re: [PATCH v6 12/17] spi: dw: Add support for AMD Pensando Elba SoC

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <2f53b4be-7e94-1941-3e33-35b14b06decc@amd.com>
Date:   Wed, 14 Sep 2022 17:03:40 +0000
From:   "Larson, Bradley" <Bradley.Larson@....com>
To:     Serge Semin <fancer.lancer@...il.com>,
        "Larson, Bradley" <Bradley.Larson@....com>
CC:     Brad Larson <brad@...sando.io>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "adrian.hunter@...el.com" <adrian.hunter@...el.com>,
        "alcooperx@...il.com" <alcooperx@...il.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "brijeshkumar.singh@....com" <brijeshkumar.singh@....com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "gsomlo@...il.com" <gsomlo@...il.com>,
        "gerg@...ux-m68k.org" <gerg@...ux-m68k.org>,
        "krzk@...nel.org" <krzk@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "yamada.masahiro@...ionext.com" <yamada.masahiro@...ionext.com>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "piotrs@...ence.com" <piotrs@...ence.com>,
        "p.yadav@...com" <p.yadav@...com>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "samuel@...lland.org" <samuel@...lland.org>,
        "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@....com>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "will@...nel.org" <will@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v6 12/17] spi: dw: Add support for AMD Pensando Elba SoC

On 9/11/22 11:20 AM, Serge Semin wrote:
> On Wed, Aug 31, 2022 at 06:04:02PM +0000, Larson, Bradley wrote:
>> On 8/21/22 11:18 AM, Serge Semin wrote:
>>> On Sat, Aug 20, 2022 at 12:57:45PM -0700, Brad Larson wrote:
>>>> From: Brad Larson <blarson@....com>
>>>>
>>>> The AMD Pensando Elba SoC includes a DW apb_ssi v4 controller
>>>> with device specific chip-select control.  The Elba SoC
>>>> provides four chip-selects where the native DW IP supports
>>>> two chip-selects.  The Elba DW_SPI instance has two native
>>>> CS signals that are always overridden.
>>>>
>>>> Signed-off-by: Brad Larson <blarson@....com>
>>>> ---
>>>>    drivers/spi/spi-dw-mmio.c | 77 +++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 77 insertions(+)
>>>>
>>>> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
>>>> index 26c40ea6dd12..36b8c5e10bb3 100644
>>>> --- a/drivers/spi/spi-dw-mmio.c
>>>> +++ b/drivers/spi/spi-dw-mmio.c
>>>> @@ -53,6 +53,24 @@ struct dw_spi_mscc {
>>>>         void __iomem        *spi_mst; /* Not sparx5 */
>>>>    };
>>>>
>>>> +struct dw_spi_elba {
>>>> +     struct regmap *syscon;
>>>> +};
>>>> +
>>>> +/*
>>>> + * Elba SoC does not use ssi, pin override is used for cs 0,1 and
>>>> + * gpios for cs 2,3 as defined in the device tree.
>>>> + *
>>>> + * cs:  |       1               0
>>>> + * bit: |---3-------2-------1-------0
>>>> + *      |  cs1   cs1_ovr   cs0   cs0_ovr
>>>> + */
>>>> +#define ELBA_SPICS_REG                       0x2468
>>>> +#define ELBA_SPICS_SHIFT(cs)         (2 * (cs))
>>>> +#define ELBA_SPICS_MASK(cs)          (0x3 << ELBA_SPICS_SHIFT(cs))
>>>> +#define ELBA_SPICS_SET(cs, val)      \
>>>> +                     ((((val) << 1) | 0x1) << ELBA_SPICS_SHIFT(cs))
>>> Please take the @Andy' notes into account:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHp75Vex0VkECYd%3DkY0m6%3DjXBYSXg2UFu7vn271%2BQ49WZn22GA%40mail.gmail.com%2F&amp;data=05%7C01%7Cbradley.larson%40amd.com%7C9e7cade823344b72f34608da94224dc6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637985172338293739%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=usQHWyOxaoD6iasP2R9VL5i0ZkSzBbdpsPljExHemfE%3D&amp;reserved=0
>> Yes, I had a tested change for this but missed adding to the patch update.
>> This is the change and I'll resend just this patch.
>>
>> --- a/drivers/spi/spi-dw-mmio.c
>> +++ b/drivers/spi/spi-dw-mmio.c
>> @@ -66,10 +66,6 @@ struct dw_spi_elba {
>>     *      |  cs1   cs1_ovr   cs0   cs0_ovr
>>     */
>>    #define ELBA_SPICS_REG 0x2468
>> -#define ELBA_SPICS_SHIFT(cs)           (2 * (cs))
>> -#define ELBA_SPICS_MASK(cs)            (0x3 << ELBA_SPICS_SHIFT(cs))
>> -#define ELBA_SPICS_SET(cs, val)        \
>> -                       ((((val) << 1) | 0x1) << ELBA_SPICS_SHIFT(cs))
> Why do you remove these macros? Just replace 0x3 with GENMASM(1, 0),
> 0x1 with BIT(0), (2 * (cs)) statement with ((cs) << 1) as Andy
> suggested. Using macros for such complex statement is a good practice.
>
> Also please rename ELBA_SPICS_SHIFT() to ELBA_SPICS_OFFSET() so to
> have a more coherent CSR-related macros naming in the driver.


Yes, will add back/rename macros with usage of BIT()/GENMASK() and 
resend just this patch.

Regards,
Brad