| Message ID | 20220422140902.1058101-1-icenowy@aosc.io |
|---|---|
| Headers | show |
| Series | Initial support for Allwinner R329 | expand |
On 4/22/22 10:40 AM, icenowy@outlook.com wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner R329 SoC has two pin controllers similar to ones on previous > SoCs, one in CPUX power domain and another in CPUS. > > This patch adds support for the CPUX domain pin controller. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> This is identical to the previous version. Please see my comments on that, as they still apply: https://lore.kernel.org/linux-sunxi/2f6de069-7983-efc2-3c4a-7c1355c4cbc5@sholland.org/
On 4/22/22 10:41 AM, icenowy@outlook.com wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner R320 SoC has a pin controller in the CPUS power domain. This should be "R329". Also see my comments on the previous version, which still apply: https://lore.kernel.org/linux-sunxi/e9937a23-8a8a-ebec-0a44-0d15a06b7e89@sholland.org/ Regards, Samuel
+RTC maintainers On 4/22/22 10:41 AM, icenowy@outlook.com wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner R329 has a RTC with a similar time storage with H616 but a > slightly different clock part. > > As we have already handled the R329 RTC clocks in the CCU driver, add a > compatible string to RTC driver to allow probing of the RTC. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> Acked-by: Samuel Holland <samuel@sholland.org> > --- > drivers/rtc/rtc-sun6i.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c > index 5b3e4da63406..522e28fb05c9 100644 > --- a/drivers/rtc/rtc-sun6i.c > +++ b/drivers/rtc/rtc-sun6i.c > @@ -816,6 +816,8 @@ static const struct of_device_id sun6i_rtc_dt_ids[] = { > { .compatible = "allwinner,sun50i-h6-rtc" }, > { .compatible = "allwinner,sun50i-h616-rtc", > .data = (void *)RTC_LINEAR_DAY }, > + { .compatible = "allwinner,sun50i-r329-rtc", > + .data = (void *)RTC_LINEAR_DAY }, > { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, sun6i_rtc_dt_ids); >
On 4/22/22 10:41 AM, icenowy@outlook.com wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner R329 has two CCUs, one in CPUX and another in PRCM. > > Add support for them. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> There is a typo in your commit title. = should be -. Thanks for updating the driver to use .fw_name and be loadable as a module. All of those changes look good. There are still some missing clocks here compared to the BSP, and a couple of other minor issues. Please see my earlier review: https://lore.kernel.org/linux-sunxi/99a74950-fdc0-ecfe-e5f0-ba4a7d8751f0@sholland.org/ So far it's been consistent that any settable bits in the CCU registers actually do something. So I would expect all of those bits to have an index reserved in the binding, even if we do not model them. I want to avoid having to go back and add gates to the binding out-of-order later, like we are doing for H6. Regards, Samuel
On 4/22/22 10:41 AM, icenowy@outlook.com wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > The two MMC controllers in Allwinner R329 have a mixed feature set > comparing to the previous SoCs' ordinary MMC and eMMC controllers. > > Add support for them. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> Acked-by: Samuel Holland <samuel@sholland.org> > --- > drivers/mmc/host/sunxi-mmc.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index 0e8fbf4957d8..06934eef8be5 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -1207,6 +1207,15 @@ static const struct sunxi_mmc_cfg sun50i_a100_emmc_cfg = { > .needs_new_timings = true, > }; > > +static const struct sunxi_mmc_cfg sun50i_r329_cfg = { > + .idma_des_size_bits = 13, > + .idma_des_shift = 2, > + .clk_delays = NULL, > + .can_calibrate = true, > + .mask_data0 = true, > + .needs_new_timings = true, > +}; > + This is the same as D1, so you could reuse or replace that configuration constant. Regards, Samuel > static const struct of_device_id sunxi_mmc_of_match[] = { > { .compatible = "allwinner,sun4i-a10-mmc", .data = &sun4i_a10_cfg }, > { .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg }, > @@ -1218,6 +1227,7 @@ static const struct of_device_id sunxi_mmc_of_match[] = { > { .compatible = "allwinner,sun50i-a64-emmc", .data = &sun50i_a64_emmc_cfg }, > { .compatible = "allwinner,sun50i-a100-mmc", .data = &sun50i_a100_cfg }, > { .compatible = "allwinner,sun50i-a100-emmc", .data = &sun50i_a100_emmc_cfg }, > + { .compatible = "allwinner,sun50i-r329-mmc", .data = &sun50i_r329_cfg }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, sunxi_mmc_of_match); >
On 4/22/22 10:41 AM, icenowy@outlook.com wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner R329 is a new SoC focused on smart audio devices. > > Add a DTSI file for it. Please make sure your devicetree passes `make dtbs_check`. There are some errors regarding the watchdog, CCU, and pinctrl. Regards, Samuel
On Fri, 22 Apr 2022 at 17:42, <icenowy@outlook.com> wrote: > > From: Icenowy Zheng <icenowy@aosc.io> > > The two MMC controllers in Allwinner R329 have a mixed feature set > comparing to the previous SoCs' ordinary MMC and eMMC controllers. > > Add support for them. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> Please repost this (and the DT patch) to linux-mmc, so I can pick them up from the patch-tracker. Kind regards Uffe > --- > drivers/mmc/host/sunxi-mmc.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index 0e8fbf4957d8..06934eef8be5 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -1207,6 +1207,15 @@ static const struct sunxi_mmc_cfg sun50i_a100_emmc_cfg = { > .needs_new_timings = true, > }; > > +static const struct sunxi_mmc_cfg sun50i_r329_cfg = { > + .idma_des_size_bits = 13, > + .idma_des_shift = 2, > + .clk_delays = NULL, > + .can_calibrate = true, > + .mask_data0 = true, > + .needs_new_timings = true, > +}; > + > static const struct of_device_id sunxi_mmc_of_match[] = { > { .compatible = "allwinner,sun4i-a10-mmc", .data = &sun4i_a10_cfg }, > { .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg }, > @@ -1218,6 +1227,7 @@ static const struct of_device_id sunxi_mmc_of_match[] = { > { .compatible = "allwinner,sun50i-a64-emmc", .data = &sun50i_a64_emmc_cfg }, > { .compatible = "allwinner,sun50i-a100-mmc", .data = &sun50i_a100_cfg }, > { .compatible = "allwinner,sun50i-a100-emmc", .data = &sun50i_a100_emmc_cfg }, > + { .compatible = "allwinner,sun50i-r329-mmc", .data = &sun50i_r329_cfg }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, sunxi_mmc_of_match); > -- > 2.35.1 >
在 2022-04-23星期六的 21:12 -0500，Samuel Holland写道： > On 4/22/22 10:41 AM, icenowy@outlook.com wrote: > > From: Icenowy Zheng <icenowy@aosc.io> > > > > Allwinner R329 has two CCUs, one in CPUX and another in PRCM. > > > > Add support for them. > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > There is a typo in your commit title. = should be -. > > Thanks for updating the driver to use .fw_name and be loadable as a > module. All > of those changes look good. > > There are still some missing clocks here compared to the BSP, and a > couple of > other minor issues. Please see my earlier review: > > https://lore.kernel.org/linux-sunxi/99a74950-fdc0-ecfe-e5f0-ba4a7d8751f0@sholland.org/ > > So far it's been consistent that any settable bits in the CCU > registers actually > do something. So I would expect all of those bits to have an index > reserved in > the binding, even if we do not model them. I want to avoid having to Sorry but I don't think it proper to reserve unclear bits, because we're just allocating the numbers as a random sequence (in fact it's the sequence that it gets implemented). Or consider a structural number scheme, in which a value can be uniquely predicted by its name? > go back and > add gates to the binding out-of-order later, like we are doing for > H6. > > Regards, > Samuel
在 2022-07-12星期二的 19:57 +0800，Icenowy Zheng写道： > 在 2022-04-23星期六的 21:12 -0500，Samuel Holland写道： > > On 4/22/22 10:41 AM, icenowy@outlook.com wrote: > > > From: Icenowy Zheng <icenowy@aosc.io> > > > > > > Allwinner R329 has two CCUs, one in CPUX and another in PRCM. > > > > > > Add support for them. > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > There is a typo in your commit title. = should be -. > > > > Thanks for updating the driver to use .fw_name and be loadable as a > > module. All > > of those changes look good. > > > > There are still some missing clocks here compared to the BSP, and a > > couple of > > other minor issues. Please see my earlier review: > > > > https://lore.kernel.org/linux-sunxi/99a74950-fdc0-ecfe-e5f0-ba4a7d8751f0@sholland.org/ > > > > So far it's been consistent that any settable bits in the CCU > > registers actually > > do something. So I would expect all of those bits to have an index > > reserved in > > the binding, even if we do not model them. I want to avoid having > > to > > Sorry but I don't think it proper to reserve unclear bits, because > we're just allocating the numbers as a random sequence (in fact it's > the sequence that it gets implemented). > > Or consider a structural number scheme, in which a value can be > uniquely predicted by its name? Well by thinking a little further, I realized our current CCU DT binding is just based on implementation details of sunxi-ng drivers (for example, some intermediate clocks get a number too, just not exposed to the DT binding header). This continues to prove that reserving numbers is just needed at all, and the current way to make CCU DT bindings might be totally wrong. Maybe we should start to use reasonable slice numbers in the next CCU driver? > > > go back and > > add gates to the binding out-of-order later, like we are doing for > > H6. > > > > Regards, > > Samuel > > >