From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755566AbeEAN3R (ORCPT ); Tue, 1 May 2018 09:29:17 -0400 Received: from fllnx210.ext.ti.com ([198.47.19.17]:42726 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754841AbeEAN3P (ORCPT ); Tue, 1 May 2018 09:29:15 -0400 Subject: Re: [PATCH v9 01/27] clk: davinci: pll: allow dev == NULL To: David Lechner , , , CC: Michael Turquette , Stephen Boyd , Rob Herring , Mark Rutland , Kevin Hilman , Bartosz Golaszewski , Adam Ford , References: <20180427001745.4116-1-david@lechnology.com> <20180427001745.4116-2-david@lechnology.com> From: Sekhar Nori Message-ID: <7f968fd3-eb0e-37ba-9f2d-2b948640ef73@ti.com> Date: Tue, 1 May 2018 18:57:53 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180427001745.4116-2-david@lechnology.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 27 April 2018 05:47 AM, David Lechner wrote: > This modifies the TI DaVinci PLL clock driver to allow for the case > when dev == NULL. On some (most) SoCs that use this driver, the PLL > clock needs to be registered during early boot because it is used > for clocksource/clockevent and there will be no platform device available. > > Signed-off-by: David Lechner > diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c > index 23a24c944f1d..7c4d808b8fdb 100644 > --- a/drivers/clk/davinci/pll.c > +++ b/drivers/clk/davinci/pll.c > @@ -111,6 +111,31 @@ struct davinci_pll_clk { > #define to_davinci_pll_clk(_hw) \ > container_of((_hw), struct davinci_pll_clk, hw) > > +static inline void *_devm_kzalloc(struct device *dev, size_t size, gfp_t flags) > +{ > + if (dev) > + return devm_kzalloc(dev, size, flags); > + > + return kzalloc(size, flags); I would shift to using kzalloc() only. The utility of devm_kzalloc() is gone if you cannot always rely on it since you have to handle the free for the other case. Same thing for other devres APIs below. > +} > + > +static inline void *_devm_kmalloc_array(struct device *dev, size_t n, > + size_t size, gfp_t flags) > +{ > + if (dev) > + return devm_kmalloc_array(dev, n, size, flags); > + > + return kmalloc_array(n, size, flags); > +} > + > +static inline struct clk *_devm_clk_register(struct device *dev, struct clk_hw *hw) > +{ > + if (dev) > + return devm_clk_register(dev, hw); > + > + return clk_register(NULL, hw); > +} > + > diff --git a/drivers/clk/davinci/pll.h b/drivers/clk/davinci/pll.h > index b1b6fb23f972..92a0978a7d29 100644 > --- a/drivers/clk/davinci/pll.h > +++ b/drivers/clk/davinci/pll.h > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > > #define PLL_HAS_CLKMODE BIT(0) /* PLL has PLLCTL[CLKMODE] */ > @@ -94,7 +95,8 @@ struct davinci_pll_obsclk_info { > struct clk *davinci_pll_clk_register(struct device *dev, > const struct davinci_pll_clk_info *info, > const char *parent_name, > - void __iomem *base); > + void __iomem *base, > + struct regmap *cfgchip); > struct clk *davinci_pll_auxclk_register(struct device *dev, > const char *name, > void __iomem *base); > @@ -110,32 +112,33 @@ davinci_pll_sysclk_register(struct device *dev, > const struct davinci_pll_sysclk_info *info, > void __iomem *base); > > -int of_davinci_pll_init(struct device *dev, > +int of_davinci_pll_init(struct device *dev, struct device_node *node, > const struct davinci_pll_clk_info *info, > const struct davinci_pll_obsclk_info *obsclk_info, > const struct davinci_pll_sysclk_info **div_info, > u8 max_sysclk_id, > - void __iomem *base); > + void __iomem *base, > + struct regmap *cfgchip); > > /* Platform-specific callbacks */ > > -int da830_pll_init(struct device *dev, void __iomem *base); > +int da830_pll_init(struct device *dev, void __iomem *base, struct regmap *cfgchip); > > -int da850_pll0_init(struct device *dev, void __iomem *base); > -int da850_pll1_init(struct device *dev, void __iomem *base); > -int of_da850_pll0_init(struct device *dev, void __iomem *base); > -int of_da850_pll1_init(struct device *dev, void __iomem *base); > +int da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip); Having this declared both here and in include/linux/clk/davinci.h is strange. Can we include that file directly where its needed? > diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h > new file mode 100644 > index 000000000000..1298cca509ac > --- /dev/null > +++ b/include/linux/clk/davinci.h > @@ -0,0 +1,24 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Clock driver for TI Davinci PSC controllers PSC/PLL controllers. Thanks, Sekhar From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sekhar Nori Subject: Re: [PATCH v9 01/27] clk: davinci: pll: allow dev == NULL Date: Tue, 1 May 2018 18:57:53 +0530 Message-ID: <7f968fd3-eb0e-37ba-9f2d-2b948640ef73@ti.com> References: <20180427001745.4116-1-david@lechnology.com> <20180427001745.4116-2-david@lechnology.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180427001745.4116-2-david@lechnology.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: David Lechner , linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: Michael Turquette , Stephen Boyd , Rob Herring , Mark Rutland , Kevin Hilman , Bartosz Golaszewski , Adam Ford , linux-kernel@vger.kernel.org List-Id: devicetree@vger.kernel.org On Friday 27 April 2018 05:47 AM, David Lechner wrote: > This modifies the TI DaVinci PLL clock driver to allow for the case > when dev == NULL. On some (most) SoCs that use this driver, the PLL > clock needs to be registered during early boot because it is used > for clocksource/clockevent and there will be no platform device available. > > Signed-off-by: David Lechner > diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c > index 23a24c944f1d..7c4d808b8fdb 100644 > --- a/drivers/clk/davinci/pll.c > +++ b/drivers/clk/davinci/pll.c > @@ -111,6 +111,31 @@ struct davinci_pll_clk { > #define to_davinci_pll_clk(_hw) \ > container_of((_hw), struct davinci_pll_clk, hw) > > +static inline void *_devm_kzalloc(struct device *dev, size_t size, gfp_t flags) > +{ > + if (dev) > + return devm_kzalloc(dev, size, flags); > + > + return kzalloc(size, flags); I would shift to using kzalloc() only. The utility of devm_kzalloc() is gone if you cannot always rely on it since you have to handle the free for the other case. Same thing for other devres APIs below. > +} > + > +static inline void *_devm_kmalloc_array(struct device *dev, size_t n, > + size_t size, gfp_t flags) > +{ > + if (dev) > + return devm_kmalloc_array(dev, n, size, flags); > + > + return kmalloc_array(n, size, flags); > +} > + > +static inline struct clk *_devm_clk_register(struct device *dev, struct clk_hw *hw) > +{ > + if (dev) > + return devm_clk_register(dev, hw); > + > + return clk_register(NULL, hw); > +} > + > diff --git a/drivers/clk/davinci/pll.h b/drivers/clk/davinci/pll.h > index b1b6fb23f972..92a0978a7d29 100644 > --- a/drivers/clk/davinci/pll.h > +++ b/drivers/clk/davinci/pll.h > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > > #define PLL_HAS_CLKMODE BIT(0) /* PLL has PLLCTL[CLKMODE] */ > @@ -94,7 +95,8 @@ struct davinci_pll_obsclk_info { > struct clk *davinci_pll_clk_register(struct device *dev, > const struct davinci_pll_clk_info *info, > const char *parent_name, > - void __iomem *base); > + void __iomem *base, > + struct regmap *cfgchip); > struct clk *davinci_pll_auxclk_register(struct device *dev, > const char *name, > void __iomem *base); > @@ -110,32 +112,33 @@ davinci_pll_sysclk_register(struct device *dev, > const struct davinci_pll_sysclk_info *info, > void __iomem *base); > > -int of_davinci_pll_init(struct device *dev, > +int of_davinci_pll_init(struct device *dev, struct device_node *node, > const struct davinci_pll_clk_info *info, > const struct davinci_pll_obsclk_info *obsclk_info, > const struct davinci_pll_sysclk_info **div_info, > u8 max_sysclk_id, > - void __iomem *base); > + void __iomem *base, > + struct regmap *cfgchip); > > /* Platform-specific callbacks */ > > -int da830_pll_init(struct device *dev, void __iomem *base); > +int da830_pll_init(struct device *dev, void __iomem *base, struct regmap *cfgchip); > > -int da850_pll0_init(struct device *dev, void __iomem *base); > -int da850_pll1_init(struct device *dev, void __iomem *base); > -int of_da850_pll0_init(struct device *dev, void __iomem *base); > -int of_da850_pll1_init(struct device *dev, void __iomem *base); > +int da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip); Having this declared both here and in include/linux/clk/davinci.h is strange. Can we include that file directly where its needed? > diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h > new file mode 100644 > index 000000000000..1298cca509ac > --- /dev/null > +++ b/include/linux/clk/davinci.h > @@ -0,0 +1,24 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Clock driver for TI Davinci PSC controllers PSC/PLL controllers. Thanks, Sekhar From mboxrd@z Thu Jan 1 00:00:00 1970 From: nsekhar@ti.com (Sekhar Nori) Date: Tue, 1 May 2018 18:57:53 +0530 Subject: [PATCH v9 01/27] clk: davinci: pll: allow dev == NULL In-Reply-To: <20180427001745.4116-2-david@lechnology.com> References: <20180427001745.4116-1-david@lechnology.com> <20180427001745.4116-2-david@lechnology.com> Message-ID: <7f968fd3-eb0e-37ba-9f2d-2b948640ef73@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Friday 27 April 2018 05:47 AM, David Lechner wrote: > This modifies the TI DaVinci PLL clock driver to allow for the case > when dev == NULL. On some (most) SoCs that use this driver, the PLL > clock needs to be registered during early boot because it is used > for clocksource/clockevent and there will be no platform device available. > > Signed-off-by: David Lechner > diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c > index 23a24c944f1d..7c4d808b8fdb 100644 > --- a/drivers/clk/davinci/pll.c > +++ b/drivers/clk/davinci/pll.c > @@ -111,6 +111,31 @@ struct davinci_pll_clk { > #define to_davinci_pll_clk(_hw) \ > container_of((_hw), struct davinci_pll_clk, hw) > > +static inline void *_devm_kzalloc(struct device *dev, size_t size, gfp_t flags) > +{ > + if (dev) > + return devm_kzalloc(dev, size, flags); > + > + return kzalloc(size, flags); I would shift to using kzalloc() only. The utility of devm_kzalloc() is gone if you cannot always rely on it since you have to handle the free for the other case. Same thing for other devres APIs below. > +} > + > +static inline void *_devm_kmalloc_array(struct device *dev, size_t n, > + size_t size, gfp_t flags) > +{ > + if (dev) > + return devm_kmalloc_array(dev, n, size, flags); > + > + return kmalloc_array(n, size, flags); > +} > + > +static inline struct clk *_devm_clk_register(struct device *dev, struct clk_hw *hw) > +{ > + if (dev) > + return devm_clk_register(dev, hw); > + > + return clk_register(NULL, hw); > +} > + > diff --git a/drivers/clk/davinci/pll.h b/drivers/clk/davinci/pll.h > index b1b6fb23f972..92a0978a7d29 100644 > --- a/drivers/clk/davinci/pll.h > +++ b/drivers/clk/davinci/pll.h > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > > #define PLL_HAS_CLKMODE BIT(0) /* PLL has PLLCTL[CLKMODE] */ > @@ -94,7 +95,8 @@ struct davinci_pll_obsclk_info { > struct clk *davinci_pll_clk_register(struct device *dev, > const struct davinci_pll_clk_info *info, > const char *parent_name, > - void __iomem *base); > + void __iomem *base, > + struct regmap *cfgchip); > struct clk *davinci_pll_auxclk_register(struct device *dev, > const char *name, > void __iomem *base); > @@ -110,32 +112,33 @@ davinci_pll_sysclk_register(struct device *dev, > const struct davinci_pll_sysclk_info *info, > void __iomem *base); > > -int of_davinci_pll_init(struct device *dev, > +int of_davinci_pll_init(struct device *dev, struct device_node *node, > const struct davinci_pll_clk_info *info, > const struct davinci_pll_obsclk_info *obsclk_info, > const struct davinci_pll_sysclk_info **div_info, > u8 max_sysclk_id, > - void __iomem *base); > + void __iomem *base, > + struct regmap *cfgchip); > > /* Platform-specific callbacks */ > > -int da830_pll_init(struct device *dev, void __iomem *base); > +int da830_pll_init(struct device *dev, void __iomem *base, struct regmap *cfgchip); > > -int da850_pll0_init(struct device *dev, void __iomem *base); > -int da850_pll1_init(struct device *dev, void __iomem *base); > -int of_da850_pll0_init(struct device *dev, void __iomem *base); > -int of_da850_pll1_init(struct device *dev, void __iomem *base); > +int da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip); Having this declared both here and in include/linux/clk/davinci.h is strange. Can we include that file directly where its needed? > diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h > new file mode 100644 > index 000000000000..1298cca509ac > --- /dev/null > +++ b/include/linux/clk/davinci.h > @@ -0,0 +1,24 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Clock driver for TI Davinci PSC controllers PSC/PLL controllers. Thanks, Sekhar