From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Mon, 5 Apr 2021 09:12:04 -0400 Subject: [PATCH v8 01/28] spi: spi-mem: allow specifying whether an op is DTR or not In-Reply-To: <20210405114754.GR1310@bill-the-cat> References: <20210401193133.18129-1-p.yadav@ti.com> <20210401193133.18129-2-p.yadav@ti.com> <20210405082504.m44kckapnn2btonl@ti.com> <20210405114754.GR1310@bill-the-cat> Message-ID: <0b352412-ab7d-713d-99e4-73e87c0af81c@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 4/5/21 7:47 AM, Tom Rini wrote: > On Mon, Apr 05, 2021 at 01:55:06PM +0530, Pratyush Yadav wrote: >> On 02/04/21 06:21PM, Sean Anderson wrote: >>> >>> On 4/1/21 3:31 PM, Pratyush Yadav wrote: >>>> Each phase is given a separate 'dtr' field so mixed protocols like >>>> 4S-4D-4D can be supported. >>>> >>>> Signed-off-by: Pratyush Yadav >>>> --- >>>> drivers/spi/spi-mem.c | 3 +++ >>>> include/spi-mem.h | 8 ++++++++ >>>> 2 files changed, 11 insertions(+) >>>> >>>> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c >>>> index c095ae9505..427f7c13c5 100644 >>>> --- a/drivers/spi/spi-mem.c >>>> +++ b/drivers/spi/spi-mem.c >>>> @@ -164,6 +164,9 @@ bool spi_mem_default_supports_op(struct spi_slave *slave, >>>> op->data.dir == SPI_MEM_DATA_OUT)) >>>> return false; >>>> + if (op->cmd.dtr || op->addr.dtr || op->dummy.dtr || op->data.dtr) >>>> + return false; >>>> + >>>> return true; >>>> } >>>> EXPORT_SYMBOL_GPL(spi_mem_default_supports_op); >>>> diff --git a/include/spi-mem.h b/include/spi-mem.h >>>> index 8be3e2bf6b..9e6b044548 100644 >>>> --- a/include/spi-mem.h >>>> +++ b/include/spi-mem.h >>>> @@ -71,6 +71,7 @@ enum spi_mem_data_dir { >>>> * struct spi_mem_op - describes a SPI memory operation >>>> * @cmd.buswidth: number of IO lines used to transmit the command >>>> * @cmd.opcode: operation opcode >>>> + * @cmd.dtr: whether the command opcode should be sent in DTR mode or not >>>> * @addr.nbytes: number of address bytes to send. Can be zero if the operation >>>> * does not need to send an address >>>> * @addr.buswidth: number of IO lines used to transmit the address cycles >>>> @@ -78,10 +79,13 @@ enum spi_mem_data_dir { >>>> * Note that only @addr.nbytes are taken into account in this >>>> * address value, so users should make sure the value fits in the >>>> * assigned number of bytes. >>>> + * @addr.dtr: whether the address should be sent in DTR mode or not >>>> * @dummy.nbytes: number of dummy bytes to send after an opcode or address. Can >>>> * be zero if the operation does not require dummy bytes >>>> * @dummy.buswidth: number of IO lanes used to transmit the dummy bytes >>>> + * @dummy.dtr: whether the dummy bytes should be sent in DTR mode or not >>>> * @data.buswidth: number of IO lanes used to send/receive the data >>>> + * @data.dtr: whether the data should be sent in DTR mode or not >>>> * @data.dir: direction of the transfer >>>> * @data.buf.in: input buffer >>>> * @data.buf.out: output buffer >>>> @@ -90,21 +94,25 @@ struct spi_mem_op { >>>> struct { >>>> u8 buswidth; >>>> u8 opcode; >>>> + u8 dtr : 1; >>>> } cmd; >>>> struct { >>>> u8 nbytes; >>>> u8 buswidth; >>>> + u8 dtr : 1; >>>> u64 val; >>>> } addr; >>>> struct { >>>> u8 nbytes; >>>> u8 buswidth; >>>> + u8 dtr : 1; >>>> } dummy; >>>> struct { >>>> u8 buswidth; >>>> + u8 dtr : 1; >>>> enum spi_mem_data_dir dir; >>>> unsigned int nbytes; >>>> /* buf.{in,out} must be DMA-able. */ >>>> >>> >>> I know this is following the Linux code, but are bitfields kosher for >>> U-Boot? This is more of a general question than a specific critique of >>> this code. >> >> I'm not sure. I did a quick grep and I do see some usages of bitfields >> but I'm not sure if these just slipped in with ports from Linux or they >> are "officially" allowed. > > We try to be a friendly environment to people used to working on the > Linux kernel, so is there a reason we wouldn't allow bitfields? > Linus has historically [1] gone on the record against the use of bitfields, primarily due to portability and optimization concerns. However, he recently [2] has condoned them for some struct types. I was just interested in what the current stance was since there isn't (e.g.) a section in process/coding_style.rst about when to use bitfields. And of course, traditionally this sort of thing would be done with a flags member. --Sean [1] https://yarchive.net/comp/linux/bitfields.html [2] https://lore.kernel.org/lkml/CA+55aFwVZk1OfB9T2v014PTAKFhtVan_Zj2dOjnCy3x6E4UJfA at mail.gmail.com/