Skip to contentSkip to conversation

spi: expand bits_per_word_mask to 64 bits

RFC In review Da Xue 5 participants 7 messages Jun 11, 2025 – Jun 12, 2025

Subsystems (7)
lore
Share & tools
lore

Da Xue

author
#0 lore raw

RFC 1/1 spi: expand bits_per_word_mask to 64 bits

Most current controller IP support 64-bit words.
Update the mask to u64 from u32.

---
 drivers/iio/adc/ad7949.c         | 2 +-
 drivers/spi/spi-dln2.c           | 2 +-
 drivers/spi/spi-ingenic.c        | 2 +-
 drivers/spi/spi-sh-msiof.c       | 2 +-
 drivers/spi/spi.c                | 4 ++--
 drivers/staging/greybus/spilib.c | 2 +-
 include/linux/spi/altera.h       | 2 +-
 include/linux/spi/spi.h          | 6 +++---
 8 files changed, 11 insertions(+), 11 deletions(-)
drivers/iio/adc/ad7949.c +1 -1
index edd0c3a35ab7..469789ffa4a3 100644
@@ -308,7 +308,7 @@ static void ad7949_disable_reg(void *reg)
 
 static int ad7949_spi_probe(struct spi_device *spi)
 {
-	u32 spi_ctrl_mask = spi->controller->bits_per_word_mask;
+	u64 spi_ctrl_mask = spi->controller->bits_per_word_mask;
 	struct device *dev = &spi->dev;
 	const struct ad7949_adc_spec *spec;
 	struct ad7949_adc_chip *ad7949_adc;
drivers/spi/spi-dln2.c +1 -1
index 2013bc56ded8..cccbb00ad5ce 100644
@@ -315,7 +315,7 @@ static int dln2_spi_set_bpw(struct dln2_spi *dln2, u8 bpw)
 }
 
 static int dln2_spi_get_supported_frame_sizes(struct dln2_spi *dln2,
-					      u32 *bpw_mask)
+					      u64 *bpw_mask)
 {
 	int ret;
 	struct {
drivers/spi/spi-ingenic.c +1 -1
index 318b0768701e..21d453d6e46e 100644
@@ -51,7 +51,7 @@
 #define SPI_INGENIC_FIFO_SIZE		128u
 
 struct jz_soc_info {
-	u32 bits_per_word_mask;
+	u64 bits_per_word_mask;
 	struct reg_field flen_field;
 	bool has_trendian;
 
drivers/spi/spi-sh-msiof.c +1 -1
index 94a867967e02..97f5a6192279 100644
@@ -33,7 +33,7 @@
 #define SH_MSIOF_FLAG_FIXED_DTDL_200	BIT(0)
 
 struct sh_msiof_chipdata {
-	u32 bits_per_word_mask;
+	u64 bits_per_word_mask;
 	u16 tx_fifo_size;
 	u16 rx_fifo_size;
 	u16 ctlr_flags;
drivers/spi/spi.c +2 -2
index 1bc0fdbb1bd7..4f47201f2462 100644
@@ -3824,8 +3824,8 @@ static int __spi_validate_bits_per_word(struct spi_controller *ctlr,
 					u8 bits_per_word)
 {
 	if (ctlr->bits_per_word_mask) {
-		/* Only 32 bits fit in the mask */
-		if (bits_per_word > 32)
+		/* Only 64 bits fit in the mask */
+		if (bits_per_word > 64)
 			return -EINVAL;
 		if (!(ctlr->bits_per_word_mask & SPI_BPW_MASK(bits_per_word)))
 			return -EINVAL;
drivers/staging/greybus/spilib.c +1 -1
index 24e9c909fa02..087eed1879b1 100644
@@ -27,7 +27,7 @@ struct gb_spilib {
 	unsigned int		op_timeout;
 	u16			mode;
 	u16			flags;
-	u32			bits_per_word_mask;
+	u64			bits_per_word_mask;
 	u8			num_chipselect;
 	u32			min_speed_hz;
 	u32			max_speed_hz;
include/linux/spi/altera.h +1 -1
index 3b74c3750caf..d6e036ed00e2 100644
@@ -24,7 +24,7 @@
 struct altera_spi_platform_data {
 	u16				mode_bits;
 	u16				num_chipselect;
-	u32				bits_per_word_mask;
+	u64				bits_per_word_mask;
 	u16				num_devices;
 	struct spi_board_info		*devices;
 };
include/linux/spi/spi.h +3 -3
index 4789f91dae94..f44120d5a63c 100644
@@ -586,7 +586,7 @@ struct spi_controller {
 	u32			buswidth_override_bits;
 
 	/* Bitmask of supported bits_per_word for transfers */
-	u32			bits_per_word_mask;
+	u64			bits_per_word_mask;
 #define SPI_BPW_MASK(bits) BIT((bits) - 1)
 #define SPI_BPW_RANGE_MASK(min, max) GENMASK((max) - 1, (min) - 1)
 
@@ -1329,9 +1329,9 @@ spi_max_transfer_size(struct spi_device *spi)
  */
 static inline bool spi_is_bpw_supported(struct spi_device *spi, u32 bpw)
 {
-	u32 bpw_mask = spi->controller->bits_per_word_mask;
+	u64 bpw_mask = spi->controller->bits_per_word_mask;
 
-	if (bpw == 8 || (bpw <= 32 && bpw_mask & SPI_BPW_MASK(bpw)))
+	if (bpw == 8 || (bpw <= 64 && bpw_mask & SPI_BPW_MASK(bpw)))
 		return true;
 
 	return false;
Signature · 1 line
2.39.5
#1 lore raw
Quoted 1 line · Da Xue

Most current controller IP support 64-bit words.

"Most of the current controllers support..."

Quoted 10 lines

Update the mask to u64 from u32.

 drivers/iio/adc/ad7949.c         | 2 +-
 drivers/spi/spi-dln2.c           | 2 +-
 drivers/spi/spi-ingenic.c        | 2 +-
 drivers/spi/spi-sh-msiof.c       | 2 +-
 drivers/spi/spi.c                | 4 ++--
 drivers/staging/greybus/spilib.c | 2 +-
 include/linux/spi/altera.h       | 2 +-
 include/linux/spi/spi.h          | 6 +++---

I guess it would be nice to split on per-driver basis, starting from updating the SPI core. I counted 6 patches in such a case.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#2 lore raw
In reply to Da Xue
Quoted 2 lines · Da Xue

Most current controller IP support 64-bit words.
Update the mask to u64 from u32.

The change looks broadly good, the only concern I have is making sure that the controllers are individually checked for support and impose any restrictions they need.

reviewer
#3 lore raw
In reply to Da Xue
Quoted 20 lines · Da Xue

Most current controller IP support 64-bit words.
Update the mask to u64 from u32.

---
 drivers/iio/adc/ad7949.c         | 2 +-
 drivers/spi/spi-dln2.c           | 2 +-
 drivers/spi/spi-ingenic.c        | 2 +-
 drivers/spi/spi-sh-msiof.c       | 2 +-
 drivers/spi/spi.c                | 4 ++--
 drivers/staging/greybus/spilib.c | 2 +-
 include/linux/spi/altera.h       | 2 +-
 include/linux/spi/spi.h          | 6 +++---
 8 files changed, 11 insertions(+), 11 deletions(-)
drivers/iio/adc/ad7949.c +0 -0
index edd0c3a35ab7..469789ffa4a3 100644

drivers/iio/adc/ad7949.c static void ad7949_disable_reg(void *reg) · line 311

Quoted 5 lines
drivers/iio/adc/ad7949.c +1 -1
@@ -308,7 +308,7 @@ static void ad7949_disable_reg(void *reg)
 
 static int ad7949_spi_probe(struct spi_device *spi)
 {
-	u32 spi_ctrl_mask = spi->controller->bits_per_word_mask;
+	u64 spi_ctrl_mask = spi->controller->bits_per_word_mask;

I think this driver is incorrectly accessing bits_per_word_mask directly and should be using spi_is_bpw_supported() instead.

This driver checks for SPI_BPW_MASK(8) at one point but doesn't take into account that if bits_per_word_mask == 0, then 8 is implied. spi_is_bpw_supported(), on the other hand, takes this into account.

Quoted 3 lines
drivers/iio/adc/ad7949.c +0 -0
 	struct device *dev = &spi->dev;
 	const struct ad7949_adc_spec *spec;
 	struct ad7949_adc_chip *ad7949_adc;

...

drivers/staging/greybus/spilib.c struct gb_spilib { · line 30

Quoted 10 lines
drivers/staging/greybus/spilib.c +1 -1
index 24e9c909fa02..087eed1879b1 100644
@@ -27,7 +27,7 @@ struct gb_spilib {
 	unsigned int		op_timeout;
 	u16			mode;
 	u16			flags;
-	u32			bits_per_word_mask;
+	u64			bits_per_word_mask;

This is assigned by:

	spi->bits_per_word_mask = le32_to_cpu(response.bits_per_word_mask);

in gb_spi_get_master_config(), so changing to u64 doesn't have any effect and should likely be omitted to avoid confusion.

(The response struct is defined by a communication protocol and can't be changed, otherwise it would break the communications.)

Quoted 3 lines
drivers/staging/greybus/spilib.c +0 -0
 	u8			num_chipselect;
 	u32			min_speed_hz;
 	u32			max_speed_hz;
maintainer
#4 lore raw
Quoted 9 lines · David Lechner

drivers/iio/adc/ad7949.c static void ad7949_disable_reg(void *reg) · line 311

Quoted 4 lines · Da Xue

static int ad7949_spi_probe(struct spi_device *spi) {

drivers/iio/adc/ad7949.c +1 -1
-	u32 spi_ctrl_mask = spi->controller->bits_per_word_mask;
+	u64 spi_ctrl_mask = spi->controller->bits_per_word_mask;

I think this driver is incorrectly accessing bits_per_word_mask directly and should be using spi_is_bpw_supported() instead.

Yes, that'd be an improvemnet.

#5 lore raw
In reply to David Lechner
Quoted 1 line · David Lechner

On 6/10/25 7:05 PM, Da Xue wrote:

...

Quoted 7 lines

Review

Quoted 4 lines

struct gb_spilib {

(quoted diff) +1 -1
-	u32			bits_per_word_mask;
+	u64			bits_per_word_mask;

This is assigned by:

Quoted 7 lines
	spi->bits_per_word_mask = le32_to_cpu(response.bits_per_word_mask);

in gb_spi_get_master_config(), so changing to u64 doesn't have any effect and should likely be omitted to avoid confusion.

(The response struct is defined by a communication protocol and can't be changed, otherwise it would break the communications.)

Perhaps the name of the field should be different to avoid appearance of the similar changes in the future (esp. if this series in general makes the upstream)?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#6 lore raw
In reply to Da Xue
kernel test robot noticed the following build warnings:

Hi Da,

[This is a private test report for your RFC patch.] kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on jic23-iio/togreg staging/staging-testing staging/staging-next staging/staging-linus linus/master v6.16-rc1 next-20250611] [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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Da-Xue/spi-expand-bits_per_word_mask-to-64-bits/20250611-104431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20250611000516.1383268-1-da%40libre.computer
patch subject: [RFC] spi: expand bits_per_word_mask to 64 bits
config: arm-randconfig-004-20250612 (https://download.01.org/0day-ci/archive/20250612/[email protected]/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250612/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/spi/spi-altera-dfl.c:150:25: warning: format specifies type 'unsigned int' but the argument has type 'u64' (aka 'unsigned long long') [-Wformat]
     149 |         dev_dbg(dev, "%s cs %u bpm 0x%x mode 0x%x\n", __func__,
         |                                      ~~
         |                                      %llx
     150 |                 host->num_chipselect, host->bits_per_word_mask,
         |                                       ^~~~~~~~~~~~~~~~~~~~~~~~

include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
171 | dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
139 | _dev_printk(level, dev, fmt, ##__VA_ARGS__); \
| ~~~ ^~~~~~~~~~~
1 warning generated.

vim +150 drivers/spi/spi-altera-dfl.c

ba2fc167e94475 Matthew Gerlach    2021-04-16  123  
ba2fc167e94475 Matthew Gerlach    2021-04-16  124  static int dfl_spi_altera_probe(struct dfl_device *dfl_dev)
ba2fc167e94475 Matthew Gerlach    2021-04-16  125  {
4f45f340496010 Martin Hundebøll   2021-07-16  126  	struct spi_board_info board_info = { 0 };
ba2fc167e94475 Matthew Gerlach    2021-04-16  127  	struct device *dev = &dfl_dev->dev;
ec168190c1cace Yang Yingliang     2022-12-29  128  	struct spi_controller *host;
ba2fc167e94475 Matthew Gerlach    2021-04-16  129  	struct altera_spi *hw;
ba2fc167e94475 Matthew Gerlach    2021-04-16  130  	void __iomem *base;
8e3ca32f46994e Christophe JAILLET 2022-05-29  131  	int err;
ba2fc167e94475 Matthew Gerlach    2021-04-16  132  
ec168190c1cace Yang Yingliang     2022-12-29  133  	host = devm_spi_alloc_host(dev, sizeof(struct altera_spi));
ec168190c1cace Yang Yingliang     2022-12-29  134  	if (!host)
ba2fc167e94475 Matthew Gerlach    2021-04-16  135  		return -ENOMEM;
ba2fc167e94475 Matthew Gerlach    2021-04-16  136  
ec168190c1cace Yang Yingliang     2022-12-29  137  	host->bus_num = -1;
ba2fc167e94475 Matthew Gerlach    2021-04-16  138  
ec168190c1cace Yang Yingliang     2022-12-29  139  	hw = spi_controller_get_devdata(host);
ba2fc167e94475 Matthew Gerlach    2021-04-16  140  
ba2fc167e94475 Matthew Gerlach    2021-04-16  141  	hw->dev = dev;
ba2fc167e94475 Matthew Gerlach    2021-04-16  142  
ba2fc167e94475 Matthew Gerlach    2021-04-16  143  	base = devm_ioremap_resource(dev, &dfl_dev->mmio_res);
ba2fc167e94475 Matthew Gerlach    2021-04-16  144  
532259bfd1c12d Zou Wei            2021-05-11  145  	if (IS_ERR(base))
ba2fc167e94475 Matthew Gerlach    2021-04-16  146  		return PTR_ERR(base);
ba2fc167e94475 Matthew Gerlach    2021-04-16  147  
ec168190c1cace Yang Yingliang     2022-12-29  148  	config_spi_host(base, host);
ba2fc167e94475 Matthew Gerlach    2021-04-16  149  	dev_dbg(dev, "%s cs %u bpm 0x%x mode 0x%x\n", __func__,
ec168190c1cace Yang Yingliang     2022-12-29 @150  		host->num_chipselect, host->bits_per_word_mask,
ec168190c1cace Yang Yingliang     2022-12-29  151  		host->mode_bits);
ba2fc167e94475 Matthew Gerlach    2021-04-16  152  
ba2fc167e94475 Matthew Gerlach    2021-04-16  153  	hw->regmap = devm_regmap_init(dev, NULL, base, &indirect_regbus_cfg);
ba2fc167e94475 Matthew Gerlach    2021-04-16  154  	if (IS_ERR(hw->regmap))
ba2fc167e94475 Matthew Gerlach    2021-04-16  155  		return PTR_ERR(hw->regmap);
ba2fc167e94475 Matthew Gerlach    2021-04-16  156  
ba2fc167e94475 Matthew Gerlach    2021-04-16  157  	hw->irq = -EINVAL;
ba2fc167e94475 Matthew Gerlach    2021-04-16  158  
ec168190c1cace Yang Yingliang     2022-12-29  159  	altera_spi_init_host(host);
ba2fc167e94475 Matthew Gerlach    2021-04-16  160  
ec168190c1cace Yang Yingliang     2022-12-29  161  	err = devm_spi_register_controller(dev, host);
8e3ca32f46994e Christophe JAILLET 2022-05-29  162  	if (err)
ec168190c1cace Yang Yingliang     2022-12-29  163  		return dev_err_probe(dev, err, "%s failed to register spi host\n",
8e3ca32f46994e Christophe JAILLET 2022-05-29  164  				     __func__);
ba2fc167e94475 Matthew Gerlach    2021-04-16  165  
4f45f340496010 Martin Hundebøll   2021-07-16  166  	if (dfl_dev->revision == FME_FEATURE_REV_MAX10_SPI_N5010)
4f45f340496010 Martin Hundebøll   2021-07-16  167  		strscpy(board_info.modalias, "m10-n5010", SPI_NAME_SIZE);
4f45f340496010 Martin Hundebøll   2021-07-16  168  	else
4f45f340496010 Martin Hundebøll   2021-07-16  169  		strscpy(board_info.modalias, "m10-d5005", SPI_NAME_SIZE);
4f45f340496010 Martin Hundebøll   2021-07-16  170  
4f45f340496010 Martin Hundebøll   2021-07-16  171  	board_info.max_speed_hz = 12500000;
4f45f340496010 Martin Hundebøll   2021-07-16  172  	board_info.bus_num = 0;
4f45f340496010 Martin Hundebøll   2021-07-16  173  	board_info.chip_select = 0;
4f45f340496010 Martin Hundebøll   2021-07-16  174  
ec168190c1cace Yang Yingliang     2022-12-29  175  	if (!spi_new_device(host, &board_info)) {
ba2fc167e94475 Matthew Gerlach    2021-04-16  176  		dev_err(dev, "%s failed to create SPI device: %s\n",
4f45f340496010 Martin Hundebøll   2021-07-16  177  			__func__, board_info.modalias);
ba2fc167e94475 Matthew Gerlach    2021-04-16  178  	}
ba2fc167e94475 Matthew Gerlach    2021-04-16  179  
ba2fc167e94475 Matthew Gerlach    2021-04-16  180  	return 0;
ba2fc167e94475 Matthew Gerlach    2021-04-16  181  }
ba2fc167e94475 Matthew Gerlach    2021-04-16  182  
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki