Re: [PATCH v4 3/6] clk: meson: eeclk: refactor eeclk common driver to support A1

[Jian Hu]

On 2019/12/12 18:19, Jerome Brunet wrote:
> On Fri 06 Dec 2019 at 08:40, Jian Hu <jian.hu@xxxxxxxxxxx> wrote:
>
> > Introduce a common probe function for A1 series, the way to get
> > regmap is different between A1 series and the previous series.
> > The register region is only for one clock driver, the function of
> > meson_eeclkc_probe is not fit for A1, So it is necessary to
> > introduce a new function.
>
> Please drop this patch
>
> #1 you are patching the EE driver for something that is no longer an EE
>   driver
> #2 Let's get the basics right, you can move (re)factoring afterward
>
> Your probe function is simple enough. Just properly write it in each
> driver for now.
OK, I will realize it in each driver.
> > Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>
> > ---
> >   drivers/clk/meson/meson-eeclk.c | 59 +++++++++++++++++++++++++++------
> >   drivers/clk/meson/meson-eeclk.h |  1 +
> >   2 files changed, 50 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/clk/meson/meson-eeclk.c b/drivers/clk/meson/meson-eeclk.c
> > index a7cb1e7aedc4..12ceb1caabd8 100644
> > --- a/drivers/clk/meson/meson-eeclk.c
> > +++ b/drivers/clk/meson/meson-eeclk.c
> > @@ -13,25 +13,37 @@
> >   #include "clk-regmap.h"
> >   #include "meson-eeclk.h"
> >   
> > -int meson_eeclkc_probe(struct platform_device *pdev)
> > +static struct regmap_config clkc_regmap_config = {
> > +	.reg_bits       = 32,
> > +	.val_bits       = 32,
> > +	.reg_stride     = 4,
> > +};
> > +
> > +static struct regmap *meson_regmap_resource(struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +	void __iomem *base;
> > +	struct device *dev = &pdev->dev;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > +	base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(base))
> > +		return ERR_CAST(base);
> > +
> > +	return devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
> > +}
> > +
> > +static int meson_common_probe(struct platform_device *pdev, struct regmap *map)
> >   {
> >   	const struct meson_eeclkc_data *data;
> >   	struct device *dev = &pdev->dev;
> > -	struct regmap *map;
> >   	int ret, i;
> >   
> >   	data = of_device_get_match_data(dev);
> >   	if (!data)
> >   		return -EINVAL;
> >   
> > -	/* Get the hhi system controller node */
> > -	map = syscon_node_to_regmap(of_get_parent(dev->of_node));
> > -	if (IS_ERR(map)) {
> > -		dev_err(dev,
> > -			"failed to get HHI regmap\n");
> > -		return PTR_ERR(map);
> > -	}
> > -
> >   	if (data->init_count)
> >   		regmap_multi_reg_write(map, data->init_regs, data->init_count);
> >   
> > @@ -54,3 +66,30 @@ int meson_eeclkc_probe(struct platform_device *pdev)
> >   	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> >   					   data->hw_onecell_data);
> >   }
> > +
> > +int meson_eeclkc_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct regmap *map;
> > +
> > +	/* Get the hhi system controller node */
> > +	map = syscon_node_to_regmap(of_get_parent(dev->of_node));
> > +	if (IS_ERR(map)) {
> > +		dev_err(dev,
> > +			"failed to get HHI regmap\n");
> > +		return PTR_ERR(map);
> > +	}
> > +
> > +	return meson_common_probe(pdev, map);
> > +}
> > +
> > +int meson_clkc_probe(struct platform_device *pdev)
> > +{
> > +	struct regmap *map;
> > +
> > +	map = meson_regmap_resource(pdev);
> > +	if (IS_ERR(map))
> > +		return PTR_ERR(map);
> > +
> > +	return meson_common_probe(pdev, map);
> > +}
> > diff --git a/drivers/clk/meson/meson-eeclk.h b/drivers/clk/meson/meson-eeclk.h
> > index 77316207bde1..a2e9ab3a4f6b 100644
> > --- a/drivers/clk/meson/meson-eeclk.h
> > +++ b/drivers/clk/meson/meson-eeclk.h
> > @@ -21,5 +21,6 @@ struct meson_eeclkc_data {
> >   };
> >   
> >   int meson_eeclkc_probe(struct platform_device *pdev);
> > +int meson_clkc_probe(struct platform_device *pdev);
> >   
> >   #endif /* __MESON_CLKC_H */
>
> .