Re: [PATCH v6 07/13] mfd: mt6370: Add MediaTek MT6370 support

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: ChiaEn Wu <peterwu.pub@gmail.com>
Cc: Lee Jones <lee.jones@linaro.org>,
	Daniel Thompson <daniel.thompson@linaro.org>,
	 Jingoo Han <jingoohan1@gmail.com>, Pavel Machek <pavel@ucw.cz>,
	Rob Herring <robh+dt@kernel.org>,
	 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	 Matthias Brugger <matthias.bgg@gmail.com>,
	Sebastian Reichel <sre@kernel.org>,
	 Chunfeng Yun <chunfeng.yun@mediatek.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	 Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Liam Girdwood <lgirdwood@gmail.com>,
	 Mark Brown <broonie@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	 "Krogerus, Heikki" <heikki.krogerus@linux.intel.com>,
	Helge Deller <deller@gmx.de>,  ChiaEn Wu <chiaen_wu@richtek.com>,
	Alice Chen <alice_chen@richtek.com>,
	 cy_huang <cy_huang@richtek.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	 Linux LED Subsystem <linux-leds@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	 linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>,
	 "moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	 USB <linux-usb@vger.kernel.org>,
	linux-iio <linux-iio@vger.kernel.org>,
	 "open list:FRAMEBUFFER LAYER" <linux-fbdev@vger.kernel.org>,
	szuni chen <szunichen@gmail.com>
Subject: Re: [PATCH v6 07/13] mfd: mt6370: Add MediaTek MT6370 support
Date: Mon, 25 Jul 2022 10:43:04 +0200	[thread overview]
Message-ID: <CAHp75VfKihBLjUFqe_Sj5dqTO7-wjLehAF+9_8-sbUeyJ-ZAmg@mail.gmail.com> (raw)
In-Reply-To: <CABtFH5++4N1mECJ0vN-79WsJJWcBTVxLFgvkiouPf1qev7LHHQ@mail.gmail.com>

On Mon, Jul 25, 2022 at 10:30 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> On Mon, Jul 25, 2022 at 4:00 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:

...

> > > +#define MT6370_REG_DEV_INFO    0x100
> > > +#define MT6370_REG_CHG_IRQ1    0x1C0
> > > +#define MT6370_REG_CHG_MASK1   0x1E0
> > > +
> > > +#define MT6370_VENID_MASK      GENMASK(7, 4)
> > > +
> > > +#define MT6370_NUM_IRQREGS     16
> > > +#define MT6370_USBC_I2CADDR    0x4E
> >
> > > +#define MT6370_REG_ADDRLEN     2
> > > +#define MT6370_REG_MAXADDR     0x1FF
> >
> > These two more logically to have near to other _REG_* definitions above.
>
> Hi Andy,
> Thanks for your review.
> Do you mean that we should move '#define MT6370_USBC_I2CADDR' and
> '#define MT6370_REG_MAXADDR' after the line '#define
> MT6370_REG_CHG_MASK1'?
> -------------------------------------------------------------------
> #define MT6370_REG_DEV_INFO    0x100
> #define MT6370_REG_CHG_IRQ1    0x1C0
> #define MT6370_REG_CHG_MASK1   0x1E0
> #define MT6370_USBC_I2CADDR    0x4E
> #define MT6370_REG_MAXADDR     0x1FF
>
> #define MT6370_VENID_MASK      GENMASK(7, 4)
>
> #define MT6370_NUM_IRQREGS     16
> #define MT6370_REG_ADDRLEN     2
> -------------------------------------------------------------------
> Like this?

You lost me. Namespace has a meaning, i.e. grouping items of a kind.
In your proposal I don't see that. If REG_MAXADDR and REG_ADDRLEN are
_not_ of the _REG_ kind as per above, why do they have this namespace
in the first place?

-- 
With Best Regards,
Andy Shevchenko

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