Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 19 Apr 2022 10:26:29 +0200
From:   Miquel Raynal <miquel.raynal@bootlin.com>
To:     Liang Yang <liang.yang@amlogic.com>
Cc:     <linux-mtd@lists.infradead.org>, Rob Herring <robh+dt@kernel.org>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Jerome Brunet <jbrunet@baylibre.com>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Kevin Hilman <khilman@baylibre.com>,
        Jianxin Pan <jianxin.pan@amlogic.com>,
        Victor Wan <victor.wan@amlogic.com>,
        XianWei Zhao <xianwei.zhao@amlogic.com>,
        Kelvin Zhang <kelvin.zhang@amlogic.com>,
        BiChao Zheng <bichao.zheng@amlogic.com>,
        YongHui Yu <yonghui.yu@amlogic.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-amlogic@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>
Subject: Re: [PATCH v4 1/2] mtd: rawnand: meson: discard the common MMC sub
 clock framework
Message-ID: <20220419102629.1c77aa2a@xps13>
In-Reply-To: <d5a33645-fac1-9c69-afe6-654bfe93ca48@amlogic.com>
References: <20220402074921.13316-1-liang.yang@amlogic.com>
        <20220402074921.13316-2-liang.yang@amlogic.com>
        <20220404103034.48ec16b1@xps13>
        <50105d6b-8ced-1b72-30cb-a709c4a4dd26@amlogic.com>
        <d5a33645-fac1-9c69-afe6-654bfe93ca48@amlogic.com>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hello,

liang.yang@amlogic.com wrote on Mon, 18 Apr 2022 11:40:10 +0800:

> Hi Miquel,
>=20
> i have some confusion when i prepare the patches. for DT compatibility, i=
t falls back to the old DT when failed to get resource by the new DT, but t=
here is some points:
> a. old DT depends on MMC sub clock driver, but it never be merged, so it =
can't work.

I don't get what you mean here, sorry. I believe there is a new way to
describe this clock but grabbing the one from the MMC still works, does
not it?

> b. if it falls back to the old DT, beside the regmap lookup below, it see=
ms that we have to preserve the code of the old clock setting in nfc_clk_in=
it().

Yes, probably.

> do we still need to avoid break DT compatibility?

We should try our best to avoid breaking the DT, yes.

>=20
> Thanks.
>=20
> On 2022/4/11 10:40, Liang Yang wrote:
> >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nfc->dev =3D dev;
> >>> -=C2=A0=C2=A0=C2=A0 res =3D platform_get_resource(pdev, IORESOURCE_ME=
M, 0);
> >>> -=C2=A0=C2=A0=C2=A0 nfc->reg_base =3D devm_ioremap_resource(dev, res);
> >>> +=C2=A0=C2=A0=C2=A0 nfc->reg_base =3D devm_platform_ioremap_resource_=
byname(pdev, "nfc"); =20
> >>
> >> This change seems unrelated. =20
> >=20
> > To be consistent with the following > devm_platform_ioremap_resource_by=
name(pdev, "emmc"). do you mean that we > don't need it?> =20
> >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (IS_ERR(nfc->reg_base))
> >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return PTR_ERR=
(nfc->reg_base);
> >>> -=C2=A0=C2=A0=C2=A0 nfc->reg_clk =3D
> >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 syscon_regmap_lookup_by_p=
handle(dev->of_node,
> >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "a=
mlogic,mmc-syscon");
> >>> -=C2=A0=C2=A0=C2=A0 if (IS_ERR(nfc->reg_clk)) {
> >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 dev_err(dev, "Failed to l=
ookup clock base\n");
> >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return PTR_ERR(nfc->reg_c=
lk);
> >>> -=C2=A0=C2=A0=C2=A0 }
> >>> +=C2=A0=C2=A0=C2=A0 nfc->sd_emmc_clock =3D devm_platform_ioremap_reso=
urce_byname(pdev, >>> "emmc");
> >>> +=C2=A0=C2=A0=C2=A0 if (IS_ERR(nfc->sd_emmc_clock))
> >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return PTR_ERR(nfc->sd_em=
mc_clock); =20
> >>
> >> While I agree this is much better than the previous solution, we cannot
> >> break DT compatibility, so you need to try getting the emmc clock, but
> >> if it fails you should fallback to the regmap lookup. =20
> >=20
> > ok, i will fix it next version. thanks.
> >  =20
> >> =20
> >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 irq =3D platform_get_irq(pdev, 0); =20


Thanks,
Miqu=C3=A8l