Add support for components requiring trailing clock after transfer
Subsystems (2)
v2 cover Add support for components requiring trailing clock after transfer
Some components require a few clock cycles with chipselect off before or/and after the data transfer done with CS on.
Typically IDT 801034 QUAD PCM CODEC datasheet states "Note *: CCLK should have one cycle before CS goes low, and two cycles after CS goes high".
The cycles "before" are implicitely provided by all previous activity on the SPI bus. But the cycles "after" must be provided in order to achieve the SPI transfer.
In order to use that kind of component, implement a new option for SPI slaves in order to implement trailing clock of a given number of bits with ChipSelect off at the end of the transfer.
This is based on a discussion we had a few years ago, see
https://lore.kernel.org/linux-spi/[email protected]/
IDT 801034 QUAD PCM CODEC datasheet can be found at
https://www.renesas.com/eu/en/document/dst/821034-data-sheet?language=en&r=24763
Changes in v2:
- Provide the number of wanted clock cycles instead of just a bool.
Christophe Leroy (2):
spi: Add optional number of additional clock cycles to be generated
spi: fsl-spi: Implement trailing bits
.../bindings/spi/spi-peripheral-props.yaml | 5 +++++ drivers/spi/spi-fsl-spi.c | 21 +++++++++++++++++-- drivers/spi/spi.c | 7 +++++-- include/linux/spi/spi.h | 1 + 4 files changed, 30 insertions(+), 4 deletions(-)
Signature · 1 line
2.34.1
PATCH v2 1/2 spi: Add optional number of additional clock cycles to be generated
Some components require a few clock cycles with chipselect off before or/and after the data transfer done with CS on.
Typically IDT 801034 QUAD PCM CODEC datasheet states "Note *: CCLK should have one cycle before CS goes low, and two cycles after CS goes high".
The cycles "before" are implicitely provided by all previous activity on the SPI bus. But the cycles "after" must be provided in order to achieve the SPI transfer.
In order to use that kind of component, implement a new option for SPI slaves in order to implement trailing clock of a given number of bits with ChipSelect off at the end of the transfer.
--- .../devicetree/bindings/spi/spi-peripheral-props.yaml | 5 +++++ drivers/spi/spi.c | 7 +++++-- include/linux/spi/spi.h | 1 + 3 files changed, 11 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml +5 -0
@@ -82,6 +82,11 @@ properties: description: Delay, in microseconds, after a write transfer. + spi-trailing-bits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Number of clock cycles with chipselect OFF after transfers. + # The controller specific properties go here. allOf: - $ref: cdns,qspi-nor-peripheral-props.yaml#
drivers/spi/spi.c +5 -2
@@ -2159,6 +2159,9 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, } } +if (!of_property_read_u32 (nc,"spi-trailing-bits" , &value)) + spi->trailing_bits = value; +if (spi_controller_is_slave (ctlr)) {if (!of_node_name_eq (nc,"slave" )) { dev_err(&ctlr->dev, "%pOF is not called 'slave'\n",
@@ -3538,14 +3541,14 @@ int spi_setup(struct spi_device *spi)trace_spi_setup (spi, status); -dev_dbg (&spi->dev,"setup mode %lu, %s%s%s%s%u bits/w, %u Hz max --> %d\n" , +dev_dbg (&spi->dev,"setup mode %lu, %s%s%s%s%u bits/w, %u Hz max, %d trailing bits --> %d\n" , spi->mode &SPI_MODE_X_MASK , (spi->mode &SPI_CS_HIGH ) ?"cs_high, " :"" , (spi->mode &SPI_LSB_FIRST ) ?"lsb, " :"" , (spi->mode &SPI_3WIRE ) ?"3wire, " :"" , (spi->mode &SPI_LOOP ) ?"loopback, " :"" , spi->bits_per_word, spi->max_speed_hz, - status); + spi->trailing_bits, status);return status; }
include/linux/spi/spi.h +1 -0
@@ -166,6 +166,7 @@ struct spi_device {u32 max_speed_hz;u8 chip_select;u8 bits_per_word; +u8 trailing_bits;bool rt;#define SPI_NO_TX BIT(31)/* no transmit wire */ #define SPI_NO_RX BIT(30)/* no receive wire */
Signature · 1 line
2.34.1
Re: [PATCH v2 1/2] spi: Add optional number of additional clock cycles to be generated
Quoted 15 lines · Christophe Leroy
Some components require a few clock cycles with chipselect off before or/and after the data transfer done with CS on.
Typically IDT 801034 QUAD PCM CODEC datasheet states "Note *: CCLK should have one cycle before CS goes low, and two cycles after CS goes high".
The cycles "before" are implicitely provided by all previous activity on the SPI bus. But the cycles "after" must be provided in order to achieve the SPI transfer.
In order to use that kind of component, implement a new option for SPI slaves in order to implement trailing clock of a given number of bits with ChipSelect off at the end of the transfer.
Quoted 7 lines
Signed-off-by Christophe Leroy <[email protected]>--- .../devicetree/bindings/spi/spi-peripheral-props.yaml | 5 +++++ drivers/spi/spi.c | 7 +++++-- include/linux/spi/spi.h | 1 + 3 files changed, 11 insertions(+), 2 deletions(-)
PATCH v2 2/2 spi: fsl-spi: Implement trailing bits
In reply to Christophe LeroyIn order to support IDT 801034 QUAD PCM CODEC, implement trailing clock for the amount of requested bits.
On fsl SPI, the minimum we can implement is a 4 bits shot. If the requested number is greated than 8, use 16.
--- drivers/spi/spi-fsl-spi.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
drivers/spi/spi-fsl-spi.c +19 -2
@@ -424,13 +424,30 @@ static int fsl_spi_do_one_msg(struct spi_master *master, } } - m->status = status; -if (status || !cs_change) {ndelay (nsecs);fsl_spi_chipselect (spi,BITBANG_CS_INACTIVE ); } +if (!status && spi->trailing_bits) { +struct spi_transfer t = { + .len =1 , + .tx_buf = empty_zero_page, + }; + +if (spi->trailing_bits <4 ) + t.bits_per_word =4 ; +else if (spi->trailing_bits >8 ) + t.bits_per_word =16 ; +else + t.bits_per_word = spi->trailing_bits; + + status =fsl_spi_setup_transfer (spi, &t); +if (!status) + status =fsl_spi_bufs (spi, &t,0 ); + } + m->status = status; +fsl_spi_setup_transfer (spi,NULL );spi_finalize_current_message (master);return 0 ;
Signature · 1 line
2.34.1
Mark Brown
maintainerRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Quoted 8 lines · Christophe Leroy
drivers/spi/spi-fsl-spi.c +8
-0+if (!status && spi->trailing_bits) { +struct spi_transfer t = { + .len =1 , + .tx_buf = empty_zero_page, + }; + +if (spi->trailing_bits <4 ) + t.bits_per_word =4 ;
drivers/spi/spi-fsl-spi.c static int fsl_spi_do_one_msg(struct spi_master *master, · line 449
Quoted 10 lines
drivers/spi/spi-fsl-spi.c +10
-0+else if (spi->trailing_bits >8 ) + t.bits_per_word =16 ; +else + t.bits_per_word = spi->trailing_bits; + + status =fsl_spi_setup_transfer (spi, &t); +if (!status) + status =fsl_spi_bufs (spi, &t,0 ); + } + m->status = status;
The binding looks good now but this is still driver specific code when it looks like it could easily be implemented in the core - like I said on the previous version you'd need to update drivers to advertise less than 8 bits but there's basically nothing driver specific I can see here so any driver using transfer_one() would get support that way.
Re: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Le 28/02/2022 à 16:29, Mark Brown a écrit :
Quoted 18 lines · Christophe Leroy
Quoted 17 lines
drivers/spi/spi-fsl-spi.c +16
-0+if (!status && spi->trailing_bits) { +struct spi_transfer t = { + .len =1 , + .tx_buf = empty_zero_page, + }; + +if (spi->trailing_bits <4 ) + t.bits_per_word =4 ; +else if (spi->trailing_bits >8 ) + t.bits_per_word =16 ; +else + t.bits_per_word = spi->trailing_bits; + + status =fsl_spi_setup_transfer (spi, &t); +if (!status) + status =fsl_spi_bufs (spi, &t,0 );
Quoted 8 lines
drivers/spi/spi-fsl-spi.c static int fsl_spi_do_one_msg(struct spi_master *master, · line 449
Quoted 2 lines
drivers/spi/spi-fsl-spi.c +2
-0+ } + m->status = status;The binding looks good now but this is still driver specific code when it looks like it could easily be implemented in the core - like I said on the previous version you'd need to update drivers to advertise less than 8 bits but there's basically nothing driver specific I can see here so any driver using
transfer_one()would get support that way.
Argh ! Sorry your comment to the previous version ended up in Junk mails. I see it now.
We discussed that back in 2016 in
https://lore.kernel.org/linux-spi/[email protected]/
and my understanding at that time was that it was not something that could be done at core level.
But maybe things have changed since then ?
By the way, fsl-spi driver doesn't implement transfer_one() but transfer_one_message() so it takes care of the chipselect changes and therefore the final dummy transfer with CS off is to be done there as far as I understand.
Would it mean changing fsl-spi driver to implement transfer_one() first ?
Thanks
Christophe
Mark Brown
maintainerRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Quoted 10 lines · Christophe Leroy
Le 28/02/2022 à 16:29, Mark Brown a écrit :
Quoted 5 lines
The binding looks good now but this is still driver specific code when it looks like it could easily be implemented in the core - like I said on the previous version you'd need to update drivers to advertise less than 8 bits but there's basically nothing driver specific I can see here so any driver using
transfer_one()would get support that way.Argh ! Sorry your comment to the previous version ended up in Junk mails. I see it now.
No problem.
Quoted 6 lines
We discussed that back in 2016 in
https://lore.kernel.org/linux-spi/[email protected]/
and my understanding at that time was that it was not something that could be done at core level.But maybe things have changed since then ?
What I said then was "it would need a new core feature" which is what the binding does, I'm suggesting that you also do that for the handling of the implementation as well.
Actually now I think about it perhaps this shouldn't be a binding at all but rather something specified by the client driver - presumably any system using an affected device is going to need these extra clock cycles so they'll all need to add the same property.
Quoted 6 lines
By the way, fsl-spi driver doesn't implement
transfer_one()buttransfer_one_message()so it takes care of the chipselect changes and therefore the final dummy transfer with CS off is to be done there as far as I understand.Would it mean changing fsl-spi driver to implement
transfer_one()first ?
Well, if it can implement transfer_one() without any negative consequences whichh
Re: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Le 28/02/2022 à 17:14, Mark Brown a écrit :
Quoted 35 lines · Christophe Leroy
Quoted 10 lines
Le 28/02/2022 à 16:29, Mark Brown a écrit :
Quoted 5 lines
The binding looks good now but this is still driver specific code when it looks like it could easily be implemented in the core - like I said on the previous version you'd need to update drivers to advertise less than 8 bits but there's basically nothing driver specific I can see here so any driver using
transfer_one()would get support that way.Argh ! Sorry your comment to the previous version ended up in Junk mails. I see it now.
No problem.
Quoted 6 lines
We discussed that back in 2016 in
https://lore.kernel.org/linux-spi/[email protected]/
and my understanding at that time was that it was not something that could be done at core level.But maybe things have changed since then ?
What I said then was "it would need a new core feature" which is what the binding does, I'm suggesting that you also do that for the handling of the implementation as well.
Actually now I think about it perhaps this shouldn't be a binding at all but rather something specified by the client driver - presumably any system using an affected device is going to need these extra clock cycles so they'll all need to add the same property.
Quoted 4 lines
By the way, fsl-spi driver doesn't implement
transfer_one()buttransfer_one_message()so it takes care of the chipselect changes and therefore the final dummy transfer with CS off is to be done there as far as I understand.
Quoted 4 lines
Quoted 1 line
Would it mean changing fsl-spi driver to implement
transfer_one()first ?Well, if it can implement
transfer_one()without any negative consequences whichh
Seems like your sentence is truncated.
My understanding today is that this trailing transfer with chipselect OFF is to be added at the end of transfer_one_message().
It can be implemented in the core transfer_one_message() for drivers implementing transfer_one(). For the other drivers not having transfer_one() but having transfer_one_message(), it must be implemented in the driver's transfer_one_message().
Am I right ?
fsl-spi driver is the one I need to support this new functionnality and it has its own transfer_one_message().
What would you expect ?
Thanks
Christophe
Mark Brown
maintainerRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Quoted 2 lines · Christophe Leroy
My understanding today is that this trailing transfer with chipselect OFF is to be added at the end of
transfer_one_message().
Yes.
Quoted 6 lines
It can be implemented in the core
transfer_one_message()for drivers implementingtransfer_one(). For the other drivers not havingtransfer_one()but havingtransfer_one_message(), it must be implemented in the driver'stransfer_one_message().Am I right ?
Yes.
Quoted 4 lines
fsl-spi driver is the one I need to support this new functionnality and it has its own
transfer_one_message().What would you expect ?
Well, if the fsl-spi driver has a good reason for open coding transfer_message() it could continue to do it, however looking at the driver it seems like it's only the little bit at the start to optimise the trasfer width in CPU mode which looks like it could easily be moved to a prepare_message() callback and save a bunch of code so it'd be good to convert it. I guess it shouldn't be strictly essential though.
Re: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
In reply to Mark BrownLe 28/02/2022 à 17:14, Mark Brown a écrit :
Quoted 8 lines
Quoted 7 lines
We discussed that back in 2016 in
https://lore.kernel.org/linux-spi/[email protected]/
and my understanding at that time was that it was not something that could be done at core level.But maybe things have changed since then ?
Quoted 8 lines
What I said then was "it would need a new core feature" which is what the binding does, I'm suggesting that you also do that for the handling of the implementation as well.
Actually now I think about it perhaps this shouldn't be a binding at all but rather something specified by the client driver - presumably any system using an affected device is going to need these extra clock cycles so they'll all need to add the same property.
Yes indeed. Therefore in v3 I took a different approach : a flag .cs_off tells to spi_transfer_one_message() that a given transfer has to be performed with chipselect OFF, therefore the consumer has full control of how and when to add those additional fake clock cycles during a transfer, and can eventually add one at anyplace during the transfer.
Here an exemple of what will do the consumer.
static int idt821034_8bit_write(struct idt821034 *idt821034, u8 val)
{
struct spi_transfer xfer[] = {{
.tx_buf = &idt821034->spi_tx_buf,
.len = 1,
},{
.tx_buf = &idt821034->spi_tx_buf,
.len = 1,
.cs_off = 1,
}};
int ret;
idt821034->spi_tx_buf = val;
ret = spi_sync_transfer(idt821034->spi, xfer, ARRAY_SIZE(xfer));
if (ret)
return ret;
return 0;
}Christophe
Mark Brown
maintainerRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Quoted 7 lines · Christophe Leroy
Yes indeed. Therefore in v3 I took a different approach : a flag .cs_off tells to
spi_transfer_one_message()that a given transfer has to be performed with chipselect OFF, therefore the consumer has full control of how and when to add those additional fake clock cycles during a transfer, and can eventually add one at anyplace during the transfer.Here an exemple of what will do the consumer.
Hrm, we should already be able to synthesize that with cs_change though there's usability challenges there and AFAICT it doesn't work for the first transfer which your proposal would so there's a functional benefit even if you don't need it for your device right now. It would be good if you could have a look at using cs_change for your use case. Sorry, I don't think I'd fully realised what you were looking to accomplish here until I saw your proposal.
Re: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Le 22/08/2022 à 19:15, Mark Brown a écrit :
Quoted 10 lines · Christophe Leroy
Quoted 8 lines
Yes indeed. Therefore in v3 I took a different approach : a flag .cs_off tells to
spi_transfer_one_message()that a given transfer has to be performed with chipselect OFF, therefore the consumer has full control of how and when to add those additional fake clock cycles during a transfer, and can eventually add one at anyplace during the transfer.Here an exemple of what will do the consumer.
Quoted 7 lines
Hrm, we should already be able to synthesize that with
cs_changethough there's usability challenges there and AFAICT it doesn't work for the first transfer which your proposal would so there's a functional benefit even if you don't need it for your device right now. It would be good if you could have a look at usingcs_changefor your use case. Sorry, I don't think I'd fully realised what you were looking to accomplish here until I saw your proposal.
I think we already addressed this possibility back in 2016, see
https://lore.kernel.org/linux-spi/[email protected]/
The conclusion was that it was not possible to accomplish it with cs_change.
Or did we miss something at that time ?
My understanding is that if you set cs_change for any transfer of a message except the last, then CS goes OFF then ON again between the said transfer and the following one. If you set cs_change for the last transfer of a message, then CS stays ON until next message instead of going OFF as usual.
My need is to transfer a fake byte with CS off at the end of a message. I still can't see how cs_change can achieve that.
Thanks
Christophe
Mark Brown
maintainerRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
Quoted 8 lines · Christophe Leroy
Le 22/08/2022 à 19:15, Mark Brown a écrit :
I think we already addressed this possibility back in 2016, see
https://lore.kernel.org/linux-spi/[email protected]/The conclusion was that it was not possible to accomplish it with
cs_change.Or did we miss something at that time ?
No, I think that's fine - your proposal has some overlap but is fine.
kernel test robot
build botRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
In reply to Christophe LeroyI love your patch! Perhaps something to improve:
Hi Christophe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on v5.17-rc6]
[cannot apply to broonie-spi/for-next next-20220228]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Add-support-for-components-requiring-trailing-clock-after-transfer/20220228-231740 base: 7e57714cd0ad2d5bb90e50b5096a0e671dec1ef3 config: hexagon-buildonly-randconfig-r004-20220228 (https://download.01.org/0day-ci/archive/20220301/[email protected]/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/7eb07c4d26401389204fcc6cf685c18c89b64ef8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/Add-support-for-components-requiring-trailing-clock-after-transfer/20220228-231740 git checkout 7eb07c4d26401389204fcc6cf685c18c89b64ef8
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/spi/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> drivers/spi/spi-fsl-spi.c:435:14: warning: incompatible integer to pointer conversion initializing 'const void *' with an expression of type 'unsigned long' [-Wint-conversion]
.tx_buf = empty_zero_page,
^~~~~~~~~~~~~~~
1 warning generated.
vim +435 drivers/spi/spi-fsl-spi.c
356
357 static int fsl_spi_do_one_msg(struct spi_master *master,
358 struct spi_message *m)
359 {
360 struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
361 struct spi_device *spi = m->spi;
362 struct spi_transfer *t, *first;
363 unsigned int cs_change;
364 const int nsecs = 50;
365 int status, last_bpw;
366
367 /*
368 * In CPU mode, optimize large byte transfers to use larger
369 * bits_per_word values to reduce number of interrupts taken.
370 */
371 if (!(mpc8xxx_spi->flags & SPI_CPM_MODE)) {
372 list_for_each_entry(t, &m->transfers, transfer_list) {
373 if (t->len < 256 || t->bits_per_word != 8)
374 continue;
375 if ((t->len & 3) == 0)
376 t->bits_per_word = 32;
377 else if ((t->len & 1) == 0)
378 t->bits_per_word = 16;
379 }
380 }
381
382 /* Don't allow changes if CS is active */
383 cs_change = 1;
384 list_for_each_entry(t, &m->transfers, transfer_list) {
385 if (cs_change)
386 first = t;
387 cs_change = t->cs_change;
388 if (first->speed_hz != t->speed_hz) {
389 dev_err(&spi->dev,
390 "speed_hz cannot change while CS is active\n");
391 return -EINVAL;
392 }
393 }
394
395 last_bpw = -1;
396 cs_change = 1;
397 status = -EINVAL;
398 list_for_each_entry(t, &m->transfers, transfer_list) {
399 if (cs_change || last_bpw != t->bits_per_word)
400 status = fsl_spi_setup_transfer(spi, t);
401 if (status < 0)
402 break;
403 last_bpw = t->bits_per_word;
404
405 if (cs_change) {
406 fsl_spi_chipselect(spi, BITBANG_CS_ACTIVE);
407 ndelay(nsecs);
408 }
409 cs_change = t->cs_change;
410 if (t->len)
411 status = fsl_spi_bufs(spi, t, m->is_dma_mapped);
412 if (status) {
413 status = -EMSGSIZE;
414 break;
415 }
416 m->actual_length += t->len;
417
418 spi_transfer_delay_exec(t);
419
420 if (cs_change) {
421 ndelay(nsecs);
422 fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
423 ndelay(nsecs);
424 }
425 }
426
427 if (status || !cs_change) {
428 ndelay(nsecs);
429 fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
430 }
431
432 if (!status && spi->trailing_bits) {
433 struct spi_transfer t = {
434 .len = 1,
> 435 .tx_buf = empty_zero_page,
436 };
437
438 if (spi->trailing_bits < 4)
439 t.bits_per_word = 4;
440 else if (spi->trailing_bits > 8)
441 t.bits_per_word = 16;
442 else
443 t.bits_per_word = spi->trailing_bits;
444
445 status = fsl_spi_setup_transfer(spi, &t);
446 if (!status)
447 status = fsl_spi_bufs(spi, &t, 0);
448 }
449 m->status = status;
450
451 fsl_spi_setup_transfer(spi, NULL);
452 spi_finalize_current_message(master);
453 return 0;
454 }
455
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
kernel test robot
build botRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
In reply to Christophe LeroyI love your patch! Perhaps something to improve:
Hi Christophe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on v5.17-rc6]
[cannot apply to broonie-spi/for-next next-20220228]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Add-support-for-components-requiring-trailing-clock-after-transfer/20220228-231740 base: 7e57714cd0ad2d5bb90e50b5096a0e671dec1ef3 config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220301/[email protected]/config) compiler: mips-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/7eb07c4d26401389204fcc6cf685c18c89b64ef8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/Add-support-for-components-requiring-trailing-clock-after-transfer/20220228-231740 git checkout 7eb07c4d26401389204fcc6cf685c18c89b64ef8
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=mips SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
drivers/spi/spi-fsl-spi.c: In function 'fsl_spi_do_one_msg':
>> drivers/spi/spi-fsl-spi.c:435:35: warning: initialization of 'const void *' from 'long unsigned int' makes pointer from integer without a cast [-Wint-conversion]
435 | .tx_buf = empty_zero_page,
| ^~~~~~~~~~~~~~~
drivers/spi/spi-fsl-spi.c:435:35: note: (near initialization for 't.tx_buf')
vim +435 drivers/spi/spi-fsl-spi.c
356
357 static int fsl_spi_do_one_msg(struct spi_master *master,
358 struct spi_message *m)
359 {
360 struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
361 struct spi_device *spi = m->spi;
362 struct spi_transfer *t, *first;
363 unsigned int cs_change;
364 const int nsecs = 50;
365 int status, last_bpw;
366
367 /*
368 * In CPU mode, optimize large byte transfers to use larger
369 * bits_per_word values to reduce number of interrupts taken.
370 */
371 if (!(mpc8xxx_spi->flags & SPI_CPM_MODE)) {
372 list_for_each_entry(t, &m->transfers, transfer_list) {
373 if (t->len < 256 || t->bits_per_word != 8)
374 continue;
375 if ((t->len & 3) == 0)
376 t->bits_per_word = 32;
377 else if ((t->len & 1) == 0)
378 t->bits_per_word = 16;
379 }
380 }
381
382 /* Don't allow changes if CS is active */
383 cs_change = 1;
384 list_for_each_entry(t, &m->transfers, transfer_list) {
385 if (cs_change)
386 first = t;
387 cs_change = t->cs_change;
388 if (first->speed_hz != t->speed_hz) {
389 dev_err(&spi->dev,
390 "speed_hz cannot change while CS is active\n");
391 return -EINVAL;
392 }
393 }
394
395 last_bpw = -1;
396 cs_change = 1;
397 status = -EINVAL;
398 list_for_each_entry(t, &m->transfers, transfer_list) {
399 if (cs_change || last_bpw != t->bits_per_word)
400 status = fsl_spi_setup_transfer(spi, t);
401 if (status < 0)
402 break;
403 last_bpw = t->bits_per_word;
404
405 if (cs_change) {
406 fsl_spi_chipselect(spi, BITBANG_CS_ACTIVE);
407 ndelay(nsecs);
408 }
409 cs_change = t->cs_change;
410 if (t->len)
411 status = fsl_spi_bufs(spi, t, m->is_dma_mapped);
412 if (status) {
413 status = -EMSGSIZE;
414 break;
415 }
416 m->actual_length += t->len;
417
418 spi_transfer_delay_exec(t);
419
420 if (cs_change) {
421 ndelay(nsecs);
422 fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
423 ndelay(nsecs);
424 }
425 }
426
427 if (status || !cs_change) {
428 ndelay(nsecs);
429 fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
430 }
431
432 if (!status && spi->trailing_bits) {
433 struct spi_transfer t = {
434 .len = 1,
> 435 .tx_buf = empty_zero_page,
436 };
437
438 if (spi->trailing_bits < 4)
439 t.bits_per_word = 4;
440 else if (spi->trailing_bits > 8)
441 t.bits_per_word = 16;
442 else
443 t.bits_per_word = spi->trailing_bits;
444
445 status = fsl_spi_setup_transfer(spi, &t);
446 if (!status)
447 status = fsl_spi_bufs(spi, &t, 0);
448 }
449 m->status = status;
450
451 fsl_spi_setup_transfer(spi, NULL);
452 spi_finalize_current_message(master);
453 return 0;
454 }
455
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
kernel test robot
build botRe: [PATCH v2 2/2] spi: fsl-spi: Implement trailing bits
In reply to Christophe LeroyI love your patch! Yet something to improve:
Hi Christophe,
I love your patch! Yet something to improve:
[auto build test ERROR on v5.17-rc6]
[cannot apply to broonie-spi/for-next next-20220228]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Add-support-for-components-requiring-trailing-clock-after-transfer/20220228-231740 base: 7e57714cd0ad2d5bb90e50b5096a0e671dec1ef3 config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220301/[email protected]/config) compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/7eb07c4d26401389204fcc6cf685c18c89b64ef8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/Add-support-for-components-requiring-trailing-clock-after-transfer/20220228-231740 git checkout 7eb07c4d26401389204fcc6cf685c18c89b64ef8
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
drivers/spi/spi-fsl-spi.c: In function 'fsl_spi_do_one_msg':
>> drivers/spi/spi-fsl-spi.c:435:35: error: 'empty_zero_page' undeclared (first use in this function)
435 | .tx_buf = empty_zero_page,
| ^~~~~~~~~~~~~~~
drivers/spi/spi-fsl-spi.c:435:35: note: each undeclared identifier is reported only once for each function it appears in
vim +/empty_zero_page +435 drivers/spi/spi-fsl-spi.c
356
357 static int fsl_spi_do_one_msg(struct spi_master *master,
358 struct spi_message *m)
359 {
360 struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
361 struct spi_device *spi = m->spi;
362 struct spi_transfer *t, *first;
363 unsigned int cs_change;
364 const int nsecs = 50;
365 int status, last_bpw;
366
367 /*
368 * In CPU mode, optimize large byte transfers to use larger
369 * bits_per_word values to reduce number of interrupts taken.
370 */
371 if (!(mpc8xxx_spi->flags & SPI_CPM_MODE)) {
372 list_for_each_entry(t, &m->transfers, transfer_list) {
373 if (t->len < 256 || t->bits_per_word != 8)
374 continue;
375 if ((t->len & 3) == 0)
376 t->bits_per_word = 32;
377 else if ((t->len & 1) == 0)
378 t->bits_per_word = 16;
379 }
380 }
381
382 /* Don't allow changes if CS is active */
383 cs_change = 1;
384 list_for_each_entry(t, &m->transfers, transfer_list) {
385 if (cs_change)
386 first = t;
387 cs_change = t->cs_change;
388 if (first->speed_hz != t->speed_hz) {
389 dev_err(&spi->dev,
390 "speed_hz cannot change while CS is active\n");
391 return -EINVAL;
392 }
393 }
394
395 last_bpw = -1;
396 cs_change = 1;
397 status = -EINVAL;
398 list_for_each_entry(t, &m->transfers, transfer_list) {
399 if (cs_change || last_bpw != t->bits_per_word)
400 status = fsl_spi_setup_transfer(spi, t);
401 if (status < 0)
402 break;
403 last_bpw = t->bits_per_word;
404
405 if (cs_change) {
406 fsl_spi_chipselect(spi, BITBANG_CS_ACTIVE);
407 ndelay(nsecs);
408 }
409 cs_change = t->cs_change;
410 if (t->len)
411 status = fsl_spi_bufs(spi, t, m->is_dma_mapped);
412 if (status) {
413 status = -EMSGSIZE;
414 break;
415 }
416 m->actual_length += t->len;
417
418 spi_transfer_delay_exec(t);
419
420 if (cs_change) {
421 ndelay(nsecs);
422 fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
423 ndelay(nsecs);
424 }
425 }
426
427 if (status || !cs_change) {
428 ndelay(nsecs);
429 fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
430 }
431
432 if (!status && spi->trailing_bits) {
433 struct spi_transfer t = {
434 .len = 1,
> 435 .tx_buf = empty_zero_page,
436 };
437
438 if (spi->trailing_bits < 4)
439 t.bits_per_word = 4;
440 else if (spi->trailing_bits > 8)
441 t.bits_per_word = 16;
442 else
443 t.bits_per_word = spi->trailing_bits;
444
445 status = fsl_spi_setup_transfer(spi, &t);
446 if (!status)
447 status = fsl_spi_bufs(spi, &t, 0);
448 }
449 m->status = status;
450
451 fsl_spi_setup_transfer(spi, NULL);
452 spi_finalize_current_message(master);
453 return 0;
454 }
455
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]