From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757914AbcASXfm (ORCPT ); Tue, 19 Jan 2016 18:35:42 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:56734 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757844AbcASXfd (ORCPT ); Tue, 19 Jan 2016 18:35:33 -0500 X-AuditID: cbfec7f5-f79b16d000005389-db-569ec841a144 Subject: Re: [PATCH v2 1/3] clk: s2mps11: allocate only one structure for clock init To: Andi Shyti , linux-samsung-soc@vger.kernel.org References: <1453193552-4761-1-git-send-email-andi.shyti@samsung.com> <1453193552-4761-2-git-send-email-andi.shyti@samsung.com> Cc: Sangbeom Kim , Michael Turquette , Stephen Boyd , linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, Andi Shyti , Yadwinder Singh Brar From: Krzysztof Kozlowski Message-id: <569EC840.1030702@samsung.com> Date: Wed, 20 Jan 2016 08:35:28 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: <1453193552-4761-2-git-send-email-andi.shyti@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrDLMWRmVeSWpSXmKPExsVy+t/xa7qOJ+aFGUx5rGex/cgzVovFP54z Wbx+YWjxseceq8XlXXPYLGac38dkcfGUq8XFFV+YLH6c6WaxmPu7kdWBy+P9jVZ2j8t9vUwe 15d8YvbYOesuu0ffllWMHp83yQWwRXHZpKTmZJalFunbJXBlzOu/zVpwTrviRjdnA+NtpS5G Dg4JAROJ3Wezuxg5gUwxiQv31rN1MXJxCAksZZT4+eMUE4TzlFHi4e9tjCBVwgJhEpt+XWMC sUUEvCSmPjzBCFHUyCgx/80rsHZmgTYmiZPvp7OAVLEJGEtsXr6EDcTmFdCSeHjvIzuIzSKg KrFqyXJmEFtUIELicGcXO0SNoMSPyffAejkF3CTWn5nEBHIqs4CexP2LWiBhZgF5ic1r3jJP YBSYhaRjFkLVLCRVCxiZVzGKppYmFxQnpeca6RUn5haX5qXrJefnbmKExMHXHYxLj1kdYhTg YFTi4WVsnhcmxJpYVlyZe4hRgoNZSYT37BagEG9KYmVValF+fFFpTmrxIUZpDhYlcd6Zu96H CAmkJ5akZqemFqQWwWSZODilGhidtJTqj5b2aTdxl39svO70uupPQ9urr28M70VPCZo4LSlT Itb1/XvWkxvfGB2LkXLumC20qLymvPyY+LraFbN3mSxd/0JqjteROR1cy223bVNZqtD9SD75 8ZeEDp97izwDX3yIzV4t+3qKQNraKE6n+bfXWVzZdehJ7T7WmVcCbLc/C6nXnNuvxFKckWio xVxUnAgA5GMMvn8CAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 19.01.2016 17:52, Andi Shyti wrote: > The driver allocates three structures for three different clock > types. They are quite similar and in the clock init data they > differ only by the name. Only one of these structures is used, > while the others lie unused in the memory. > > The clock's name, though, is not such a meaningful information > and by assigning the same name to the initial data we can avoid > over allocation. The common name chosen will be s2mps11, > coherently with the device driver name, instead of the clock > device. > > Therefore, remove the structures associated to s2mps13 and > s2mps14 and use only the one referred to s2mps11 for all kind of > clocks. > > Signed-off-by: Andi Shyti > Suggested-by: Krzysztof Kozlowski > --- > drivers/clk/clk-s2mps11.c | 56 ++++++----------------------------------------- > 1 file changed, 7 insertions(+), 49 deletions(-) > > diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c > index d266299..22d4df2 100644 > --- a/drivers/clk/clk-s2mps11.c > +++ b/drivers/clk/clk-s2mps11.c > @@ -99,6 +99,7 @@ static struct clk_ops s2mps11_clk_ops = { > .recalc_rate = s2mps11_clk_recalc_rate, > }; > > +/* This s2mps11_clks_init tructure is common to s2mps11, s2mps13 and s2mps14 */ > static struct clk_init_data s2mps11_clks_init[S2MPS11_CLKS_NUM] = { > [S2MPS11_CLK_AP] = { > .name = "s2mps11_ap", > @@ -117,43 +118,11 @@ static struct clk_init_data s2mps11_clks_init[S2MPS11_CLKS_NUM] = { > }, > }; > > -static struct clk_init_data s2mps13_clks_init[S2MPS11_CLKS_NUM] = { > - [S2MPS11_CLK_AP] = { > - .name = "s2mps13_ap", > - .ops = &s2mps11_clk_ops, > - .flags = CLK_IS_ROOT, > - }, > - [S2MPS11_CLK_CP] = { > - .name = "s2mps13_cp", > - .ops = &s2mps11_clk_ops, > - .flags = CLK_IS_ROOT, > - }, > - [S2MPS11_CLK_BT] = { > - .name = "s2mps13_bt", > - .ops = &s2mps11_clk_ops, > - .flags = CLK_IS_ROOT, > - }, > -}; > - > -static struct clk_init_data s2mps14_clks_init[S2MPS11_CLKS_NUM] = { > - [S2MPS11_CLK_AP] = { > - .name = "s2mps14_ap", > - .ops = &s2mps11_clk_ops, > - .flags = CLK_IS_ROOT, > - }, > - [S2MPS11_CLK_BT] = { > - .name = "s2mps14_bt", > - .ops = &s2mps11_clk_ops, > - .flags = CLK_IS_ROOT, > - }, > -}; > - > static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev, > struct clk_init_data *clks_init) > { > struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent); > struct device_node *clk_np; > - int i; > > if (!iodev->dev->of_node) > return ERR_PTR(-EINVAL); > @@ -164,13 +133,6 @@ static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev, > return ERR_PTR(-EINVAL); > } > > - for (i = 0; i < S2MPS11_CLKS_NUM; i++) { > - if (!clks_init[i].name) > - continue; /* Skip clocks not present in some devices */ > - of_property_read_string_index(clk_np, "clock-output-names", i, > - &clks_init[i].name); > - } > - The commit description does not mention anything about removal of "clock-output-names". Why you are removing this? Best regards, Krzysztof > return clk_np; > } > > @@ -179,8 +141,8 @@ static int s2mps11_clk_probe(struct platform_device *pdev) > struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent); > struct s2mps11_clk *s2mps11_clks, *s2mps11_clk; > unsigned int s2mps11_reg; > - struct clk_init_data *clks_init; > int i, ret = 0; > + enum sec_device_type hwid = platform_get_device_id(pdev)->driver_data; > > s2mps11_clks = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM, > sizeof(*s2mps11_clk), GFP_KERNEL); > @@ -194,22 +156,18 @@ static int s2mps11_clk_probe(struct platform_device *pdev) > if (!clk_table) > return -ENOMEM; > > - switch(platform_get_device_id(pdev)->driver_data) { > + switch (hwid) { > case S2MPS11X: > s2mps11_reg = S2MPS11_REG_RTC_CTRL; > - clks_init = s2mps11_clks_init; > break; > case S2MPS13X: > s2mps11_reg = S2MPS13_REG_RTCCTRL; > - clks_init = s2mps13_clks_init; > break; > case S2MPS14X: > s2mps11_reg = S2MPS14_REG_RTCCTRL; > - clks_init = s2mps14_clks_init; > break; > case S5M8767X: > s2mps11_reg = S5M8767_REG_CTRL1; > - clks_init = s2mps11_clks_init; > break; > default: > dev_err(&pdev->dev, "Invalid device type\n"); > @@ -217,15 +175,15 @@ static int s2mps11_clk_probe(struct platform_device *pdev) > } > > /* Store clocks of_node in first element of s2mps11_clks array */ > - s2mps11_clks->clk_np = s2mps11_clk_parse_dt(pdev, clks_init); > + s2mps11_clks->clk_np = s2mps11_clk_parse_dt(pdev, s2mps11_clks_init); > if (IS_ERR(s2mps11_clks->clk_np)) > return PTR_ERR(s2mps11_clks->clk_np); > > for (i = 0; i < S2MPS11_CLKS_NUM; i++, s2mps11_clk++) { > - if (!clks_init[i].name) > + if (i == S2MPS11_CLK_CP && hwid == S2MPS14X) > continue; /* Skip clocks not present in some devices */ > s2mps11_clk->iodev = iodev; > - s2mps11_clk->hw.init = &clks_init[i]; > + s2mps11_clk->hw.init = &s2mps11_clks_init[i]; > s2mps11_clk->mask = 1 << i; > s2mps11_clk->reg = s2mps11_reg; > > @@ -248,7 +206,7 @@ static int s2mps11_clk_probe(struct platform_device *pdev) > > for (i = 0; i < S2MPS11_CLKS_NUM; i++) { > /* Skip clocks not present on S2MPS14 */ > - if (!clks_init[i].name) > + if (i == S2MPS11_CLK_CP && hwid == S2MPS14X) > continue; > clk_table[i] = s2mps11_clks[i].clk; > } >