linux-kernel - Re: [PATCH v3 0/6] iio: accel: adxl345: Add spi-3wire feature

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 24 Mar 2024 20:20:21 +0100
From: Lothar Rubusch <l.rubusch@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, robh+dt@...nel.org, 
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, 
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, eraretuya@...il.com
Subject: Re: [PATCH v3 0/6] iio: accel: adxl345: Add spi-3wire feature

On Sun, Mar 24, 2024 at 2:39 PM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Sat, 23 Mar 2024 12:20:24 +0000
> Lothar Rubusch <l.rubusch@...il.com> wrote:
>
> > Pass a function setup() as pointer from SPI/I2C specific modules
> > to the core module. Implement setup() to pass the spi-3wire bus
> > option, if declared in the device-tree.
> >
> > In the core module, then update data_format register
> > configuration bits instead of overwriting it. The changes allow
> > to remove a data_range field, remove I2C and SPI redundant info
> > instances and replace them by a common info array instance.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>
> That patch break up seems reasonable (one minor request for a split
> in the relevant patch), but normal convention would be do do
> refactoring first, then functionality at the end. Also removal stuff
> and group, before adding things.
>
> So roughly speaking reorder as
>
> >   iio: accel: adxl345: Make data_format obsolete
> >   iio: accel: adxl345: Remove single info instances
> >   iio: accel: adxl345: Group bus configuration
> >   dt-bindings: iio: accel: adxl345: Add spi-3wire
> >   iio: accel: adxl345: Pass function pointer to core
> >   iio: accel: adxl345: Add the spi-3wire
>

Ok. If I split "Group bus configuration" into the grouping of the
indio_dev in the probe() and adding a comment to the core's probe(), I
will end up with something like this:

$ git log --oneline --reverse
 iio: accel: adxl345: Make data_range obsolete
 iio: accel: adxl345: Group bus configuration
 iio: accel: adxl345: Move defines to header <--- new
 dt-bindings: iio: accel: adxl345: Add spi-3wire
 iio: accel: adxl345: Pass function pointer to core
 iio: accel: adxl345: Add comment to probe  <--- new after split
 iio: accel: adxl345: Add spi-3wire option

I feel I have to add the comment after adding the passed function
pointer. Bascially I liked to add a comment mentioning especially the
new function pointer there. So, although being a comment, the commit
will be in this "high" position. Is this ok, or am I doing something
wrong? Should I split into setting the comment first, then inside
"Pass function pointer.." also update the comment?