All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: "Heiner Kallweit" <hkallweit1@gmail.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Amlogic Meson..."
	<linux-amlogic@lists.infradead.org>,
	linux-pwm@vger.kernel.org
Subject: Re: [PATCH v3 3/4] pwm: meson: change clk/pwm gate from mask to bit
Date: Thu, 13 Apr 2023 11:06:44 +0200	[thread overview]
Message-ID: <ZDfGJM24xZbfSur5@orome> (raw)
In-Reply-To: <CAFBinCDs=EQo8-HSSbaprfJB+93sz+Ng1H=MX3hBG_00PTko3g@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1709 bytes --]

On Wed, Apr 12, 2023 at 10:47:05PM +0200, Martin Blumenstingl wrote:
> Hi Heiner,
> 
> On Wed, Apr 12, 2023 at 9:23 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >
> > Change single-bit values from mask to bit. This facilitates
> > CCF initialization for the clock gate in a follow-up patch.
> >
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> #
> meson8b-odroidc1, sm1-x96-air
> 
> [...]
> >  #define REG_MISC_AB            0x8
> > -#define MISC_B_CLK_EN          BIT(23)
> > -#define MISC_A_CLK_EN          BIT(15)
> > +#define MISC_B_CLK_EN          23
> > +#define MISC_A_CLK_EN          15
> >  #define MISC_CLK_DIV_MASK      0x7f
> >  #define MISC_B_CLK_DIV_SHIFT   16
> >  #define MISC_A_CLK_DIV_SHIFT   8
> >  #define MISC_B_CLK_SEL_SHIFT   6
> >  #define MISC_A_CLK_SEL_SHIFT   4
> >  #define MISC_CLK_SEL_MASK      0x3
> > -#define MISC_B_EN              BIT(1)
> > -#define MISC_A_EN              BIT(0)
> > +#define MISC_B_EN              1
> > +#define MISC_A_EN              0
> Personally I'm fine with this change but it's not how I would have done it:
> - I would have kept the BIT() macro for MISC_{A,B}_EN
> - then I would have renamed MISC_{A,}_CLK_EN to
> MISC_{A,B}_CLK_EN_SHIFT (to be consistent with _SHIFT of the mux and
> divider) and drop the BIT() macro there (like you did)
> 
> This is possibly/likely personal preference, so my suggestion is to
> wait for some more feedback.

It looks like these aren't used outside the meson_pwm_per_channel_data
array, so why bother with a #define (and any potential inconsistencies)
in the first place?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: "Heiner Kallweit" <hkallweit1@gmail.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Amlogic Meson..."
	<linux-amlogic@lists.infradead.org>,
	linux-pwm@vger.kernel.org
Subject: Re: [PATCH v3 3/4] pwm: meson: change clk/pwm gate from mask to bit
Date: Thu, 13 Apr 2023 11:06:44 +0200	[thread overview]
Message-ID: <ZDfGJM24xZbfSur5@orome> (raw)
In-Reply-To: <CAFBinCDs=EQo8-HSSbaprfJB+93sz+Ng1H=MX3hBG_00PTko3g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

On Wed, Apr 12, 2023 at 10:47:05PM +0200, Martin Blumenstingl wrote:
> Hi Heiner,
> 
> On Wed, Apr 12, 2023 at 9:23 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >
> > Change single-bit values from mask to bit. This facilitates
> > CCF initialization for the clock gate in a follow-up patch.
> >
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> #
> meson8b-odroidc1, sm1-x96-air
> 
> [...]
> >  #define REG_MISC_AB            0x8
> > -#define MISC_B_CLK_EN          BIT(23)
> > -#define MISC_A_CLK_EN          BIT(15)
> > +#define MISC_B_CLK_EN          23
> > +#define MISC_A_CLK_EN          15
> >  #define MISC_CLK_DIV_MASK      0x7f
> >  #define MISC_B_CLK_DIV_SHIFT   16
> >  #define MISC_A_CLK_DIV_SHIFT   8
> >  #define MISC_B_CLK_SEL_SHIFT   6
> >  #define MISC_A_CLK_SEL_SHIFT   4
> >  #define MISC_CLK_SEL_MASK      0x3
> > -#define MISC_B_EN              BIT(1)
> > -#define MISC_A_EN              BIT(0)
> > +#define MISC_B_EN              1
> > +#define MISC_A_EN              0
> Personally I'm fine with this change but it's not how I would have done it:
> - I would have kept the BIT() macro for MISC_{A,B}_EN
> - then I would have renamed MISC_{A,}_CLK_EN to
> MISC_{A,B}_CLK_EN_SHIFT (to be consistent with _SHIFT of the mux and
> divider) and drop the BIT() macro there (like you did)
> 
> This is possibly/likely personal preference, so my suggestion is to
> wait for some more feedback.

It looks like these aren't used outside the meson_pwm_per_channel_data
array, so why bother with a #define (and any potential inconsistencies)
in the first place?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: "Heiner Kallweit" <hkallweit1@gmail.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Amlogic Meson..."
	<linux-amlogic@lists.infradead.org>,
	linux-pwm@vger.kernel.org
Subject: Re: [PATCH v3 3/4] pwm: meson: change clk/pwm gate from mask to bit
Date: Thu, 13 Apr 2023 11:06:44 +0200	[thread overview]
Message-ID: <ZDfGJM24xZbfSur5@orome> (raw)
In-Reply-To: <CAFBinCDs=EQo8-HSSbaprfJB+93sz+Ng1H=MX3hBG_00PTko3g@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1709 bytes --]

On Wed, Apr 12, 2023 at 10:47:05PM +0200, Martin Blumenstingl wrote:
> Hi Heiner,
> 
> On Wed, Apr 12, 2023 at 9:23 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >
> > Change single-bit values from mask to bit. This facilitates
> > CCF initialization for the clock gate in a follow-up patch.
> >
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> #
> meson8b-odroidc1, sm1-x96-air
> 
> [...]
> >  #define REG_MISC_AB            0x8
> > -#define MISC_B_CLK_EN          BIT(23)
> > -#define MISC_A_CLK_EN          BIT(15)
> > +#define MISC_B_CLK_EN          23
> > +#define MISC_A_CLK_EN          15
> >  #define MISC_CLK_DIV_MASK      0x7f
> >  #define MISC_B_CLK_DIV_SHIFT   16
> >  #define MISC_A_CLK_DIV_SHIFT   8
> >  #define MISC_B_CLK_SEL_SHIFT   6
> >  #define MISC_A_CLK_SEL_SHIFT   4
> >  #define MISC_CLK_SEL_MASK      0x3
> > -#define MISC_B_EN              BIT(1)
> > -#define MISC_A_EN              BIT(0)
> > +#define MISC_B_EN              1
> > +#define MISC_A_EN              0
> Personally I'm fine with this change but it's not how I would have done it:
> - I would have kept the BIT() macro for MISC_{A,B}_EN
> - then I would have renamed MISC_{A,}_CLK_EN to
> MISC_{A,B}_CLK_EN_SHIFT (to be consistent with _SHIFT of the mux and
> divider) and drop the BIT() macro there (like you did)
> 
> This is possibly/likely personal preference, so my suggestion is to
> wait for some more feedback.

It looks like these aren't used outside the meson_pwm_per_channel_data
array, so why bother with a #define (and any potential inconsistencies)
in the first place?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-04-13  9:07 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-12 19:18 [PATCH v3 0/4] pwm: meson: make full use of common clock framework Heiner Kallweit
2023-04-12 19:18 ` Heiner Kallweit
2023-04-12 19:18 ` Heiner Kallweit
2023-04-12 19:20 ` [PATCH v3 1/4] pwm: meson: switch to using struct clk_parent_data for mux parents Heiner Kallweit
2023-04-12 19:20   ` Heiner Kallweit
2023-04-12 19:20   ` Heiner Kallweit
2023-04-12 20:40   ` Martin Blumenstingl
2023-04-12 20:40     ` Martin Blumenstingl
2023-04-12 20:40     ` Martin Blumenstingl
2023-04-12 19:21 ` [PATCH v3 2/4] pwm: meson: don't use hdmi/video clock as mux parent Heiner Kallweit
2023-04-12 19:21   ` Heiner Kallweit
2023-04-12 19:21   ` Heiner Kallweit
2023-04-12 20:42   ` Martin Blumenstingl
2023-04-12 20:42     ` Martin Blumenstingl
2023-04-12 20:42     ` Martin Blumenstingl
2023-04-12 19:21 ` [PATCH v3 3/4] pwm: meson: change clk/pwm gate from mask to bit Heiner Kallweit
2023-04-12 19:21   ` Heiner Kallweit
2023-04-12 19:21   ` Heiner Kallweit
2023-04-12 20:47   ` Martin Blumenstingl
2023-04-12 20:47     ` Martin Blumenstingl
2023-04-12 20:47     ` Martin Blumenstingl
2023-04-13  9:06     ` Thierry Reding [this message]
2023-04-13  9:06       ` Thierry Reding
2023-04-13  9:06       ` Thierry Reding
2023-04-13 21:17       ` Heiner Kallweit
2023-04-13 21:17         ` Heiner Kallweit
2023-04-13 21:17         ` Heiner Kallweit
2023-04-12 19:23 ` [PATCH v3 4/4] pwm: meson: make full use of common clock framework Heiner Kallweit
2023-04-12 19:23   ` Heiner Kallweit
2023-04-12 19:23   ` Heiner Kallweit
2023-04-12 21:05   ` Martin Blumenstingl
2023-04-12 21:05     ` Martin Blumenstingl
2023-04-12 21:05     ` Martin Blumenstingl
2023-04-12 21:51     ` Heiner Kallweit
2023-04-12 21:51       ` Heiner Kallweit
2023-04-12 21:51       ` Heiner Kallweit
2023-04-13  6:11     ` Heiner Kallweit
2023-04-13  6:11       ` Heiner Kallweit
2023-04-13  6:11       ` Heiner Kallweit

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=ZDfGJM24xZbfSur5@orome \
    --to=thierry.reding@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=narmstrong@baylibre.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.