Re: [PATCH v3 2/3] mfd: axp20x: Add support for AXP15060 PMIC

From: Lee Jones
Date: Mon May 15 2023 - 06:52:39 EST


On Thu, 04 May 2023, Krzysztof Kozlowski wrote:

> On 03/05/2023 13:07, Andre Przywara wrote:
> > On Wed, 26 Apr 2023 15:27:40 +0100
> > Lee Jones <lee@xxxxxxxxxx> wrote:
> >
> > Hi Lee,
> >
> > I see this patch in Linus' tree, but something must have gone wrong here,
> > can you please check? See below ...
> >
> >> On Fri, 21 Apr 2023, Shengyu Qu wrote:
> >>
> >>> The AXP15060 is a PMIC chip produced by X-Powers, and could be connected
> >>> via an I2C bus.
> >>>
> >>> Describe the regmap and the MFD bits, along with the registers exposed
> >>> via I2C. Eventually advertise the device using a new compatible string
> >>> and add support for power off the system.
> >>>
> >>> The driver would disable PEK function if IRQ is not configured in device
> >>> tree, since some boards (For example, Starfive Visionfive 2) didn't
> >>> connect IRQ line of PMIC to SOC.
> >>>
> >>> GPIO function isn't enabled in this commit, since its configuration
> >>> operation is different from any existing AXP PMICs and needs
> >>> logic modification on existing driver. GPIO support might come in later
> >>> patches.
> >>>
> >>> ---
> >>
> >> You must not use these above the tags or Git will drop them.
> >>
> >>> Changes since v2:
> >>> - Rebase to AXP313a series v10 [1] + newest (20230420) -next branch
> >
> > So this patch was based on the AXP313a series, but I don't see that in
> > Linus' tree (or in any of your trees, if I have checked correctly).
> > There must have been a conflict, as this [PATCH v3 2/3] diff actually lists
> > the axp313a entry in the context lines.
> >
> >>> - Add axp_regulator_only_cells rather than directly using axp806_cells
> >>> for cases that IRQ line isn't connected.
> >>>
> >>> Changes since v1:
> >>> - Nothing
> >>>
> >>> [1] https://lore.kernel.org/linux-sunxi/20230401001850.4988-1-andre.przywara@xxxxxxx/
> >>>
> >>> Signed-off-by: Shengyu Qu <wiagn233@xxxxxxxxxxx>
> >>> ---
> >>
> >> Put change-logs here instead.
> >>
> >>> drivers/mfd/axp20x-i2c.c | 2 +
> >>> drivers/mfd/axp20x.c | 107 +++++++++++++++++++++++++++++++++++++
> >>> include/linux/mfd/axp20x.h | 85 +++++++++++++++++++++++++++++
> >>> 3 files changed, 194 insertions(+)
> >>
> >> I manually added the missing tags for this and the DT patch and applied.
> >
> > So this patch doesn't list any tags aside from Shengyu's
> > Signed-off-by. The patch in Linus' tree list a Reviewed-by: from
> > Krzysztof, which I don't see anywhere in the thread, he just reviewed the
> > binding patch, AFAICT.
>
> Yep, I never reviewed this.
>
> > I see your tentative R-b: on v2, but with the
> > request to rebase and resend, which he did with v3. The applied patch
> > looks like v3, but not on the base commit this was send against.
> >
> > So I am slightly confused, and am also wondering what happened to the
> > AXP313a patches? I see the binding patch merged, but not the MFD part,
> > even though you replied saying so.
>
> Because the patch #1 was broken, see:
> https://lore.kernel.org/all/TY3P286MB261177CF7AA2959BD9517DA998609@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>
> The SoB and Reviewed-by were after --- and apparently b4 understood it
> as cover letter and applied everywhere.
>
> Lee,
> Do you have the latest b4? If yes, this should be reported as b4 bug,
> assuming you used it.

I am using b4, although the version I'm using is quite old (0.9.0).

Also, this was quite some time ago - I have slept since applying this
and do not distinctly remember doing so. Thus, the application of your
R-b may well have been a mistake on my part. I'll keep an eye for such
things in the future and if I see (and remember) an issue, I'll report
it.

--
Lee Jones [李琼斯]