| Message ID | 20190606164443.6991-1-festevam@gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | clk: imx8qxp: Use devm_platform_ioremap_resource() | expand |
Quoting Fabio Estevam (2019-06-06 09:44:43) > Use devm_platform_ioremap_resource() to simplify the code a bit. > > Signed-off-by: Fabio Estevam <festevam@gmail.com> > --- Reviewed-by: Stephen Boyd <sboyd@kernel.org>
On 06.06.2019 19:55, Stephen Boyd wrote: > Quoting Fabio Estevam (2019-06-06 09:44:43) >> Use devm_platform_ioremap_resource() to simplify the code a bit. >> >> Signed-off-by: Fabio Estevam <festevam@gmail.com> >> --- > > Reviewed-by: Stephen Boyd <sboyd@kernel.org> An extremely similar patch was already submitted and then reverted because it breaks boot: https://patchwork.kernel.org/patch/10908807/ I tested and this new patch also breaks boot. The current imx8 lpcg driver maps entire subsystems at once and if devm_platform_ioremap_resource is used then devices inside the subsystem will fail to probe, including lpuart! The hardware on imx8qxp and related parts (imx8qm) has multiple separate LPCG blocks interspersed between devices. Some refactoring patches were posted by Aisheng to split LPCG into multiple blocks but apparently got stuck in review: https://patchwork.kernel.org/cover/10924029/ https://patchwork.kernel.org/cover/10824443/ https://patchwork.kernel.org/cover/10824537/ There were some disagreements regarding DT bindings for split imx8 clocks, what would it take to move those patches forward? -- Regards, Leonard
Quoting Leonard Crestez (2019-06-06 10:15:32) > On 06.06.2019 19:55, Stephen Boyd wrote: > > Quoting Fabio Estevam (2019-06-06 09:44:43) > >> Use devm_platform_ioremap_resource() to simplify the code a bit. > >> > >> Signed-off-by: Fabio Estevam <festevam@gmail.com> > >> --- > > > > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > > An extremely similar patch was already submitted and then reverted > because it breaks boot: > > https://patchwork.kernel.org/patch/10908807/ > > I tested and this new patch also breaks boot. > > The current imx8 lpcg driver maps entire subsystems at once and if > devm_platform_ioremap_resource is used then devices inside the subsystem > will fail to probe, including lpuart! > > The hardware on imx8qxp and related parts (imx8qm) has multiple separate > LPCG blocks interspersed between devices. Some refactoring patches were > posted by Aisheng to split LPCG into multiple blocks but apparently got > stuck in review: > > https://patchwork.kernel.org/cover/10924029/ > https://patchwork.kernel.org/cover/10824443/ > https://patchwork.kernel.org/cover/10824537/ > > There were some disagreements regarding DT bindings for split imx8 > clocks, what would it take to move those patches forward? > Don't know. I'll have to read those patches on the list and reply there.
On 06.06.2019 20:22, Stephen Boyd wrote: > Quoting Leonard Crestez (2019-06-06 10:15:32) >> On 06.06.2019 19:55, Stephen Boyd wrote: >>> Quoting Fabio Estevam (2019-06-06 09:44:43) >>>> Use devm_platform_ioremap_resource() to simplify the code a bit. >>>> >>>> Signed-off-by: Fabio Estevam <festevam@gmail.com> >>>> --- >>> >>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> >> >> An extremely similar patch was already submitted and then reverted >> because it breaks boot: >> >> The current imx8 lpcg driver maps entire subsystems at once and if >> devm_platform_ioremap_resource is used then devices inside the subsystem >> will fail to probe, including lpuart! >> >> The hardware on imx8qxp and related parts (imx8qm) has multiple separate >> LPCG blocks interspersed between devices. Some refactoring patches were >> posted by Aisheng to split LPCG into multiple blocks but apparently got >> stuck in review: >> >> There were some disagreements regarding DT bindings for split imx8 >> clocks, what would it take to move those patches forward? > > Don't know. I'll have to read those patches on the list and reply there. Some specific advice on how to refactor and split imx8qxp clk in a way that is acceptable to upstream would be very helpful to us. -- Regards, Leonard
On Thu, Jun 6, 2019 at 2:15 PM Leonard Crestez <leonard.crestez@nxp.com> wrote: > An extremely similar patch was already submitted and then reverted > because it breaks boot: > > https://patchwork.kernel.org/patch/10908807/ > > I tested and this new patch also breaks boot. Ops, sorry about that!
diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c index fb6edf1b8aa2..716af92242d2 100644 --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c @@ -159,7 +159,6 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev) struct clk_hw_onecell_data *clk_data; const struct imx8qxp_ss_lpcg *ss_lpcg; const struct imx8qxp_lpcg_data *lpcg; - struct resource *res; struct clk_hw **clks; void __iomem *base; int i; @@ -168,12 +167,9 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev) if (!ss_lpcg) return -ENODEV; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) - return -EINVAL; - base = devm_ioremap(dev, res->start, resource_size(res)); - if (!base) - return -ENOMEM; + base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(base)) + return PTR_ERR(base); clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_data, hws, ss_lpcg->num_max), GFP_KERNEL);
Use devm_platform_ioremap_resource() to simplify the code a bit. Signed-off-by: Fabio Estevam <festevam@gmail.com> --- drivers/clk/imx/clk-imx8qxp-lpcg.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)