mbox series

[v1,0/2] spi: Introduce and use spi_bpw_to_bytes()

Message ID 20250416062013.1826421-1-andriy.shevchenko@linux.intel.com
Headers show
Series spi: Introduce and use spi_bpw_to_bytes() | expand

Message

Andy Shevchenko April 16, 2025, 6:16 a.m. UTC
Recently in the discussion with David the idea of having
a common helper popped up. The helper converts the given
bits per word to bytes. The result will always be power-of-two
(e.g. for 37 bits it returns 8 bytes) or 0 for 0 input.

This mini-series introduces it and replaces current users
under drivers/spi and we expect more (and possibly some lurking
in other subsystems).

Mark, if you okay with the idea, please, make this to be an immutable
branch or tag for others to pull.

Andy Shevchenko (2):
  spi: Add spi_bpw_to_bytes() helper and use it
  spi: dw: Use spi_bpw_to_bytes() helper

 drivers/spi/spi-dw-core.c |  2 +-
 drivers/spi/spi.c         |  2 +-
 include/linux/spi/spi.h   | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

Comments

Andy Shevchenko April 16, 2025, 7:09 a.m. UTC | #1
On Wed, Apr 16, 2025 at 12:33:24PM +0530, Mukesh Kumar Savaliya wrote:
> On 4/16/2025 11:46 AM, Andy Shevchenko wrote:

Thanks for the prompt review, my answers below.

...

> > +/**
> > + * spi_bpw_to_bytes - Covert bits per word to bytes
> > + * @bpw: Bits per word
> > + *
> > + * This function converts the given @bpw to bytes. The result is always
> > + * power-of-two (e.g. for 37 bits it returns 8 bytes) or 0 for 0 input.
> Would it be good to say in 4 byte aligned /Multiples ?

It's not correct. The said wording describes the current behaviour.

> > + * Returns:
> > + * Bytes for the given @bpw.
> Returns: Bytes for the given @bpw.
> Good to keep in one line.

Aligned with the style of the other function in the same header, so I prefer to
leave the style the same.

> > + */> +static inline u32 spi_bpw_to_bytes(u32 bpw)
> u8 bpw ?

Nope. See below why.

> struct spi_device {
> u8 bits_per_word;
> }

> so arg should be u8.

It's aligned with the above bpw related function.
Also note, that this helper might be moved to the global header at some point
as some other subsystems may utilise it, so I don't want to limit this to u8.
Andy Shevchenko April 16, 2025, 3:14 p.m. UTC | #2
On Wed, Apr 16, 2025 at 09:56:12AM -0500, David Lechner wrote:
> On 4/16/25 1:16 AM, Andy Shevchenko wrote:

...

> > +/**
> > + * spi_bpw_to_bytes - Covert bits per word to bytes
> > + * @bpw: Bits per word
> > + *
> > + * This function converts the given @bpw to bytes. The result is always
> > + * power-of-two (e.g. for 37 bits it returns 8 bytes) or 0 for 0 input.
> 
> The SPI subsystem currently only supports bpw up to 32, so perhaps not
> the best choice of value for the example. I would go with 20 bits getting
> rounded up to 4 bytes to match the existing docs for @bits_per_word.

Okay, I think I come up with a few examples, so it will show that it's not
4-byte multiple or so.

> > + * Returns:
> > + * Bytes for the given @bpw.
> > + */
> > +static inline u32 spi_bpw_to_bytes(u32 bpw)
> > +{
> > +	return roundup_pow_of_two(BITS_TO_BYTES(bpw));

> Do we need to #include <linux/log2.h> for roundup_pow_of_two()?

Right now I prefer not to touch that (it is implicitly included); the headers
needs to have a bigger cleanup and my first attempt had miserably failed.

> > +}
Mark Brown April 18, 2025, 5:25 a.m. UTC | #3
On Wed, 16 Apr 2025 09:16:33 +0300, Andy Shevchenko wrote:
> Recently in the discussion with David the idea of having
> a common helper popped up. The helper converts the given
> bits per word to bytes. The result will always be power-of-two
> (e.g. for 37 bits it returns 8 bytes) or 0 for 0 input.
> 
> This mini-series introduces it and replaces current users
> under drivers/spi and we expect more (and possibly some lurking
> in other subsystems).
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/2] spi: Add spi_bpw_to_bytes() helper and use it
      commit: 163ddf1fea590229c30a8dc4c29ff4febfb895c3
[2/2] spi: dw: Use spi_bpw_to_bytes() helper
      commit: e30b7a75666b3f444abfabed6a144642fa9994d8

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark