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
| ||
|
Date: Wed, 25 May 2022 14:54:47 -0700 From: Brad Larson <brad@...sando.io> To: Serge Semin <fancer.lancer@...il.com> Cc: Serge Semin <Sergey.Semin@...kalelectronics.ru>, Linux ARM <linux-arm-kernel@...ts.infradead.org>, Arnd Bergmann <arnd@...db.de>, Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski <bgolaszewski@...libre.com>, Mark Brown <broonie@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, Ulf Hansson <ulf.hansson@...aro.org>, Olof Johansson <olof@...om.net>, David Clear <dac2@...sando.io>, "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>, linux-spi <linux-spi@...r.kernel.org>, linux-mmc <linux-mmc@...r.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [PATCH 10/11] spi: dw: Add support for Pensando Elba SoC Hi Sergey, On Tue, Apr 12, 2022 at 4:06 AM Serge Semin <fancer.lancer@...il.com> wrote: > > > +static void elba_spics_set_cs(struct dw_spi_elba *dwselba, int cs, int enable) > > +{ > > + regmap_update_bits(dwselba->regmap, dwselba->reg, ELBA_SPICS_MASK(cs), > > + ELBA_SPICS_SET(cs, enable)); > > +} > > + > > +static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable) > > The methods naming is ambiguous. Moreover it breaks this module naming > convention. Could you change them to something like: > dw_spi_elba_override_cs() > and > dw_spi_elba_set_cs() > ? Yes, changed elba_spics_set_cs() -> dw_spi_elba_override_cs() > > + /* deassert cs */ > > > + elba_spics_set_cs(dwselba, 0, 1); > > + elba_spics_set_cs(dwselba, 1, 1); > > What if the CS lines are of the active-high type? In that case basically > you get to do the opposite to what you claim in the comment here. > > Note the CS setting into the deactivated state is done in the spi_setup() > method anyway, at the moment of the peripheral SPI device registration > stage (see its calling the spi_set_cs() function). Thus what you are doing > here is redundant. Yes this is a redundant initialization and is removed. For Elba these CS lines are active-low only. Regards, Brad
Powered by blists - more mailing lists