| 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
| ||
|
Message-ID: <182aa3a1fcd05a2e25b55442f58ced5b@risingedge.co.za>
Date: Sat, 16 Mar 2024 03:01:02 +0200
From: Justin Swartz <justin.swartz@...ingedge.co.za>
To: Mark Brown <broonie@...nel.org>, Matthias Brugger
<matthias.bgg@...il.com>, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
Cc: linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] spi: mt7621: allow GPIO chip select lines
Please ignore this patch. It was accidentally sent without
"v2" nor --in-reply-to=...
On 2024-03-16 02:59, Justin Swartz wrote:
> Extract a magic number, from mt7621_spi_probe(), used to
> declare the number of chip select lines (which co-incides
> with the native chip select count of 2) to a macro.
>
> Use the newly defined MT7621_NATIVE_CS_COUNT macro to
> instead populate both the spi_controller's max_native_cs
> and num_chipselect members.
>
> Declare that the spi_controller should use_gpio_descriptors
> if present in the device properties (such as those declared
> in the cs-gpio property of a "ralink,mt7621-spi" compatible
> device-tree node) so that the SPI core will recalculcate
> num_chipselect to account for the GPIO descriptors that
> it should have populated in the cs_gpiod array member.
>
> Remove the assignment of mt7621_spi_transfer_one_message()
> to the spi_controller's transfer_one_message hook.
>
> Refactor the mt7621_spi_transfer_one_message() logic into
> mt7621_spi_prepare_message() and mt7621_spi_transfer_one()
> and assign both to the spi_controller's prepare_message
> and transfer_one hooks respectively.
>
> Migrate the call mt7621_spi_transfer_one_message() made to
> mt7621_spi_flush() just before chip select deactivation,
> to the end of mt7621_spi_write_half_duplex() to ensure
> that any pending data is shifted out of MOSI before the SPI
> core deactivates the chip select line.
>
> As chip select activation is now taken care of by the SPI
> core, due to the use of the transfer_one hook instead of
> transfer_one_message, the calls to mt7621_spi_set_cs()
> from mt7621_spi_transfer_one_message() have fallen away.
>
> And although the SPI core will handle activation for GPIO
> chip select lines behind the scenes, it requires a callback
> to allow the driver to perform controller-specific
> operations to control its native chip select lines.
>
> Rename mt7621_spi_set_cs() to mt7621_spi_set_native_cs()
> and make sure that it takes into account the activation
> polarity of the chip select line it's acting upon, as the
> passed enable parameter represents the desired line level
> and not the desired activation state, and then assign
> mt7621_set_cs() to the spi_controller's set_cs hook.
>
> Signed-off-by: Justin Swartz <justin.swartz@...ingedge.co.za>
> ---
>
> Changes from v1 to v2:
>
> Mark Brown recommended using the transfer_one hook
> approach, so I did.
>
>
> drivers/spi/spi-mt7621.c | 95 +++++++++++++++++++---------------------
> 1 file changed, 45 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c
> index 4e9053d03..3770b8e09 100644
> --- a/drivers/spi/spi-mt7621.c
> +++ b/drivers/spi/spi-mt7621.c
> @@ -52,6 +52,8 @@
> #define MT7621_CPOL BIT(4)
> #define MT7621_LSB_FIRST BIT(3)
>
> +#define MT7621_NATIVE_CS_COUNT 2
> +
> struct mt7621_spi {
> struct spi_controller *host;
> void __iomem *base;
> @@ -75,10 +77,11 @@ static inline void mt7621_spi_write(struct
> mt7621_spi *rs, u32 reg, u32 val)
> iowrite32(val, rs->base + reg);
> }
>
> -static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
> +static void mt7621_spi_set_native_cs(struct spi_device *spi, bool
> enable)
> {
> struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
> int cs = spi_get_chipselect(spi, 0);
> + bool active = spi->mode & SPI_CS_HIGH ? enable : !enable;
> u32 polar = 0;
> u32 host;
>
> @@ -94,7 +97,7 @@ static void mt7621_spi_set_cs(struct spi_device
> *spi, int enable)
>
> rs->pending_write = 0;
>
> - if (enable)
> + if (active)
> polar = BIT(cs);
> mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
> }
> @@ -154,6 +157,23 @@ static inline int
> mt7621_spi_wait_till_ready(struct mt7621_spi *rs)
> return -ETIMEDOUT;
> }
>
> +static int mt7621_spi_prepare_message(struct spi_controller *host,
> + struct spi_message *m)
> +{
> + struct mt7621_spi *rs = spi_controller_get_devdata(host);
> + struct spi_device *spi = m->spi;
> + unsigned int speed = spi->max_speed_hz;
> + struct spi_transfer *t = NULL;
> +
> + mt7621_spi_wait_till_ready(rs);
> +
> + list_for_each_entry(t, &m->transfers, transfer_list)
> + if (t->speed_hz < speed)
> + speed = t->speed_hz;
> +
> + return mt7621_spi_prepare(spi, speed);
> +}
> +
> static void mt7621_spi_read_half_duplex(struct mt7621_spi *rs,
> int rx_len, u8 *buf)
> {
> @@ -243,59 +263,30 @@ static void mt7621_spi_write_half_duplex(struct
> mt7621_spi *rs,
> }
>
> rs->pending_write = len;
> + mt7621_spi_flush(rs);
> }
>
> -static int mt7621_spi_transfer_one_message(struct spi_controller
> *host,
> - struct spi_message *m)
> +static int mt7621_spi_transfer_one(struct spi_controller *host,
> + struct spi_device *spi,
> + struct spi_transfer *t)
> {
> struct mt7621_spi *rs = spi_controller_get_devdata(host);
> - struct spi_device *spi = m->spi;
> - unsigned int speed = spi->max_speed_hz;
> - struct spi_transfer *t = NULL;
> - int status = 0;
> -
> - mt7621_spi_wait_till_ready(rs);
>
> - list_for_each_entry(t, &m->transfers, transfer_list)
> - if (t->speed_hz < speed)
> - speed = t->speed_hz;
> -
> - if (mt7621_spi_prepare(spi, speed)) {
> - status = -EIO;
> - goto msg_done;
> - }
> -
> - /* Assert CS */
> - mt7621_spi_set_cs(spi, 1);
> -
> - m->actual_length = 0;
> - list_for_each_entry(t, &m->transfers, transfer_list) {
> - if ((t->rx_buf) && (t->tx_buf)) {
> - /*
> - * This controller will shift some extra data out
> - * of spi_opcode if (mosi_bit_cnt > 0) &&
> - * (cmd_bit_cnt == 0). So the claimed full-duplex
> - * support is broken since we have no way to read
> - * the MISO value during that bit.
> - */
> - status = -EIO;
> - goto msg_done;
> - } else if (t->rx_buf) {
> - mt7621_spi_read_half_duplex(rs, t->len, t->rx_buf);
> - } else if (t->tx_buf) {
> - mt7621_spi_write_half_duplex(rs, t->len, t->tx_buf);
> - }
> - m->actual_length += t->len;
> + if ((t->rx_buf) && (t->tx_buf)) {
> + /*
> + * This controller will shift some extra data out
> + * of spi_opcode if (mosi_bit_cnt > 0) &&
> + * (cmd_bit_cnt == 0). So the claimed full-duplex
> + * support is broken since we have no way to read
> + * the MISO value during that bit.
> + */
> + return -EIO;
> + } else if (t->rx_buf) {
> + mt7621_spi_read_half_duplex(rs, t->len, t->rx_buf);
> + } else if (t->tx_buf) {
> + mt7621_spi_write_half_duplex(rs, t->len, t->tx_buf);
> }
>
> - /* Flush data and deassert CS */
> - mt7621_spi_flush(rs);
> - mt7621_spi_set_cs(spi, 0);
> -
> -msg_done:
> - m->status = status;
> - spi_finalize_current_message(host);
> -
> return 0;
> }
>
> @@ -353,10 +344,14 @@ static int mt7621_spi_probe(struct
> platform_device *pdev)
> host->mode_bits = SPI_LSB_FIRST;
> host->flags = SPI_CONTROLLER_HALF_DUPLEX;
> host->setup = mt7621_spi_setup;
> - host->transfer_one_message = mt7621_spi_transfer_one_message;
> + host->prepare_message = mt7621_spi_prepare_message;
> + host->set_cs = mt7621_spi_set_native_cs;
> + host->transfer_one = mt7621_spi_transfer_one;
> host->bits_per_word_mask = SPI_BPW_MASK(8);
> host->dev.of_node = pdev->dev.of_node;
> - host->num_chipselect = 2;
> + host->max_native_cs = MT7621_NATIVE_CS_COUNT;
> + host->num_chipselect = MT7621_NATIVE_CS_COUNT;
> + host->use_gpio_descriptors = true;
>
> dev_set_drvdata(&pdev->dev, host);
Powered by blists - more mailing lists