linux-kernel - Re: [PATCH v2 13/14] spi: bcmbca-hsspi: Add driver for newer HSSPI controller

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]

Date:   Thu, 26 Jan 2023 18:17:58 -0800
From:   William Zhang <william.zhang@...adcom.com>
To:     Jonas Gorski <jonas.gorski@...il.com>
Cc:     Linux SPI List <linux-spi@...r.kernel.org>,
        Broadcom Kernel List <bcm-kernel-feedback-list@...adcom.com>,
        tomer.yacoby@...adcom.com, kursad.oney@...adcom.com,
        dregan@...l.com, f.fainelli@...il.com, anand.gore@...adcom.com,
        dan.beygelman@...adcom.com, joel.peshkin@...adcom.com,
        kernel test robot <lkp@...el.com>,
        Mark Brown <broonie@...nel.org>,
        Rafał Miłecki <rafal@...ecki.pl>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 13/14] spi: bcmbca-hsspi: Add driver for newer HSSPI
 controller



On 01/26/2023 07:16 AM, Jonas Gorski wrote:
>> +static void bcmbca_hsspi_set_cs(struct bcmbca_hsspi *bs, unsigned int cs,
>> +                                bool active)
>> +{
>> +       u32 reg;
>> +
>> +       /* No cs orerriden needed for SS7 internal cs on pcm based voice dev */
>> +       if (cs == 7)
>> +               return;
>> +
>> +       mutex_lock(&bs->bus_mutex);
>> +
>> +       if (active) {
>> +               /* activate cs by setting the override bit and active value bit*/
>> +               reg = __raw_readl(bs->spim_ctrl);
>> +               reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
> 
> Doesn't checkpatch complain about missing spaces around the operator (+)?
> 
Nope...

>> +               reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
>> +               if (bs->cs_polarity & BIT(cs))
>> +                       reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
> 
> Does the CS_OVERRIDE_VAL get reset on clearing the OVERRIDE_SEL bit or
> it is persistent? If the latter, you could initialize its value in
> bcmbca_hsspi_setup() and then this becomes:
> 
> reg = _raw_readl(bs->spim_ctrl);
> if (active)
>     reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
> else
>     reg &= ~BIT(cs + SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
>   __raw_writel(reg, bs->spim_ctrl);
> 
> and you can drop the cs_polarity field.
> 
> 
No. So I will try this optimization and see if that actually works in 
the controller.

>> +               __raw_writel(reg, bs->spim_ctrl);
>> +       } else {
>> +               /* clear the cs override bit */
>> +               reg = __raw_readl(bs->spim_ctrl);
>> +               reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
>> +               __raw_writel(reg, bs->spim_ctrl);
>> +       }
>> +
>> +       mutex_unlock(&bs->bus_mutex);
>> +}
>> +

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4212 bytes)