public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Lothar Rubusch <l.rubusch@gmail.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>,
	lars@metafoo.de, Michael.Hennerich@analog.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, eraretuya@gmail.com
Subject: Re: [PATCH v6 2/7] iio: accel: adxl345: complete the list of defines
Date: Sat, 14 Dec 2024 11:49:14 +0000	[thread overview]
Message-ID: <20241214114914.02a280dd@jic23-huawei> (raw)
In-Reply-To: <CAFXKEHZr2MT6Ard2pTpQtU9BVrr8FHes0wFO0PU=rM7iFX6H8A@mail.gmail.com>

On Thu, 12 Dec 2024 10:37:55 +0100
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Hi  Krzysztof,
> Thank you so much for reviewing.
> 
> On Thu, Dec 12, 2024 at 9:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Wed, Dec 11, 2024 at 11:06:43PM +0000, Lothar Rubusch wrote:  
> > > Extend the list of constants. Keep them the header file for readability.
> > > The defines allow the implementation of events like watermark, single
> > > tap, double tap, freefall, etc.  
> >
> > We don't store constants just to store constants, so this commit does
> > not have reason to be separate. We store constants/defines only to
> > implement the driver. Merge these with the users... unless you want to
> > say there are no users of this at all, but then make it clear: move the
> > patch to the end.
> >  
> 
> I see your point.
> 
> The defines are needed for the current introduction of the FIFO usage,
> connected with the watermark feature. Some of it is related to
> upcoming features, such as mentioned in the comment (tap events,
> freefall, powersafe, selftest, etc).
> 
> This patch series now on FIFO/watermark are just the first step to get
> a solid reviewed common base. Further features are upcoming. I did not
> split up the constants. All the specified registers will be needed to
> allow for their configuration and setup. I understand it's no organig
> growth by immediate need, as I understand, but giving IMHO a bit
> flexibility then in implementing what is the next feature, since all
> registers are already defined.
> 
> Pls, let me know, if you prefer me to only introduce immediately
> needed constants for a current specific feature?

That would be the normal way to do it in a series that is adding those
features.

There are cases where we do blanket includes of all registers etc in 
one patch but they tend to be autogenerated from another source (so
annoying to split up) rather than introduced alongside features.

Also tends to be more common for first posting of a driver rather than
adding new features when the author of the driver decided to do a subset
(so follow the local style).

Jonathan

> Best,
> L
> 
> > Best regards,
> > Krzysztof
> >  


  reply	other threads:[~2024-12-14 11:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-11 23:06 [PATCH v6 0/7] iio: accel: adxl345: add FIFO operating with IRQ triggered watermark events Lothar Rubusch
2024-12-11 23:06 ` [PATCH v6 1/7] iio: accel: adxl345: add function to switch measuring mode Lothar Rubusch
2024-12-11 23:06 ` [PATCH v6 2/7] iio: accel: adxl345: complete the list of defines Lothar Rubusch
2024-12-12  8:07   ` Krzysztof Kozlowski
2024-12-12  9:37     ` Lothar Rubusch
2024-12-14 11:49       ` Jonathan Cameron [this message]
2024-12-11 23:06 ` [PATCH v6 3/7] dt-bindings: iio: accel: adxl345: add interrupt-names Lothar Rubusch
2024-12-12  8:08   ` Krzysztof Kozlowski
2024-12-14 11:56     ` Jonathan Cameron
2024-12-14 11:59   ` Jonathan Cameron
2024-12-11 23:06 ` [PATCH v6 4/7] dt-bindings: iio: accel: adxl345: make interrupts not a required property Lothar Rubusch
2024-12-12  8:11   ` Krzysztof Kozlowski
2024-12-13  8:06     ` Lothar Rubusch
2024-12-13  8:49       ` Krzysztof Kozlowski
2024-12-11 23:06 ` [PATCH v6 5/7] iio: accel: adxl345: introduce interrupt handling Lothar Rubusch
2024-12-11 23:06 ` [PATCH v6 6/7] iio: accel: adxl345: initialize FIFO delay value for SPI Lothar Rubusch
2024-12-11 23:06 ` [PATCH v6 7/7] iio: accel: adxl345: add FIFO with watermark events Lothar Rubusch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241214114914.02a280dd@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=eraretuya@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=l.rubusch@gmail.com \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox