Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum

From: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org, linux-pwm@vger.kernel.org,
	"Paul Barker" <pbarker@konsulko.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Marcel Ziswiler" <marcel.ziswiler@toradex.com>,
	"Igor Opaniuk" <igor.opaniuk@toradex.com>,
	"Philippe Schenker" <philippe.schenker@toradex.com>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Claudiu Beznea" <claudiu.beznea@microchip.com>,
	"Fabio Estevam" <festevam@gmail.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Ludovic Desroches" <ludovic.desroches@microchip.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Paul Cercueil" <paul@crapouillou.net>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Ray Jui" <rjui@broadcom.com>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Scott Branden" <sbranden@broadcom.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Tony Prisk" <linux@prisktech.co.nz>,
	bcm-kernel-feedback-list@broadcom.com,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Subject: Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
Date: Wed, 18 Mar 2020 13:47:40 +0200	[thread overview]
Message-ID: <CAGgjyvGDTD=ZgEjU540HLApQXCoM1a8bk_-eQbS44sge+GpeAg@mail.gmail.com> (raw)
In-Reply-To: <20200317174043.GA1464607@ulmo>

On Tue, Mar 17, 2020 at 7:41 PM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > Rename it to PWM_POLARITY_INVERTED.
>
> It isn't misspelled. "inversed" is a synonym for "inverted". Both
> spellings are correct.

> And as you noted in the cover letter, there's a conflict between the
> macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> in the wrong order you'll get a compile error.

This patch is a part of the patchset, which in result removes enum at all,
so there will be no definition conflict.

> The enum was named this way on purpose to make it separate from the
> definition for the DT bindings. Note that DT bindings are an ABI and can
> never change, whereas the enum pwm_polarity is part of a Linux internal
> API and doesn't have the same restrictions as an ABI.

AFAIU, DTS files are not a part of ABI.

I understand that enums are better than macros for some reasons.
However, I think it is dangerous to use duplicate definitions in
different places when values of these definitions use in the same
code.
So, given that the enum cannot be used in DT, I left only macros.

You personally wrote that the enum pwm_polarity can change, so the
desynchronization I quite possible.

> As far as I'm concerned this is completely unnecessary churn that's
> potentially going to come back and bite us, so I see no reason to accept
> this.

>
> Thierry
--
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00