Re: [PATCH 3/4] phy: cadence-torrent: Do not configure SERDES if it's already configured

2021-03-30 Thread Kishon Vijay Abraham I 
Hi Swapnil,

On 18/03/21 3:25 pm, Swapnil Kashinath Jakhade wrote:
> 
> 
>> -Original Message-
>> From: Kishon Vijay Abraham I 
>> Sent: Wednesday, March 10, 2021 9:25 PM
>> To: Kishon Vijay Abraham I ; Vinod Koul
>> ; Rob Herring ; Philipp Zabel
>> ; Swapnil Kashinath Jakhade
>> 
>> Cc: linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; Lokesh Vutla
>> ; linux-...@lists.infradead.org
>> Subject: [PATCH 3/4] phy: cadence-torrent: Do not configure SERDES if it's
>> already configured
>>
>> EXTERNAL MAIL
>>
>>
>> Do not configure torrent SERDES if it's already configured.
>>
>> Signed-off-by: Kishon Vijay Abraham I 
>> ---
>>  drivers/phy/cadence/phy-cadence-torrent.c | 32 ---
>>  1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/phy/cadence/phy-cadence-torrent.c
>> b/drivers/phy/cadence/phy-cadence-torrent.c
>> index ab51c4bf7b30..5ee1657f5a1c 100644
>> --- a/drivers/phy/cadence/phy-cadence-torrent.c
>> +++ b/drivers/phy/cadence/phy-cadence-torrent.c
>> @@ -371,6 +371,10 @@ static const struct phy_ops cdns_torrent_phy_ops =
>> {
>>  .owner  = THIS_MODULE,
>>  };
>>
>> +static const struct phy_ops noop_ops = {
>> +.owner  = THIS_MODULE,
>> +};
>> +
>>  struct cdns_reg_pairs {
>>  u32 val;
>>  u32 off;
>> @@ -2306,6 +2310,7 @@ static int cdns_torrent_phy_probe(struct
>> platform_device *pdev)
>>  struct device_node *child;
>>  int ret, subnodes, node = 0, i;
>>  u32 total_num_lanes = 0;
>> +int already_configured;
>>  u8 init_dp_regmap = 0;
>>  u32 phy_type;
>>
>> @@ -2344,16 +2349,20 @@ static int cdns_torrent_phy_probe(struct
>> platform_device *pdev)
>>  if (ret)
>>  return ret;
>>
>> -ret = cdns_torrent_reset(cdns_phy);
>> -if (ret)
>> -goto clk_cleanup;
>> +regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1,
>> _configured);
>>
>> -ret = cdns_torrent_clk(cdns_phy);
>> -if (ret)
>> -goto clk_cleanup;
>> +if (!already_configured) {
>> +ret = cdns_torrent_reset(cdns_phy);
>> +if (ret)
>> +goto clk_cleanup;
>> +
>> +ret = cdns_torrent_clk(cdns_phy);
>> +if (ret)
>> +goto clk_cleanup;
>>
> Should if (!already_configured) be checked while calling 
> clk_disable_unprepare()?

Both clk_unprepare() and clk_disable() checks for passed clk for ERR or
NULL. So don't think it's necessary to have !already_configured check.
> Otherwise,
> Reviewed-by: Swapnil Jakhade 

Thanks
Kishon

> 
> Thanks & regards,
> Swapnil
> 
>> -/* Enable APB */
>> -reset_control_deassert(cdns_phy->apb_rst);
>> +/* Enable APB */
>> +reset_control_deassert(cdns_phy->apb_rst);
>> +}
>>
>>  for_each_available_child_of_node(dev->of_node, child) {
>>  struct phy *gphy;
>> @@ -2423,7 +2432,10 @@ static int cdns_torrent_phy_probe(struct
>> platform_device *pdev)
>>  of_property_read_u32(child, "cdns,ssc-mode",
>>   _phy->phys[node].ssc_mode);
>>
>> -gphy = devm_phy_create(dev, child,
>> _torrent_phy_ops);
>> +if (!already_configured)
>> +gphy = devm_phy_create(dev, child,
>> _torrent_phy_ops);
>> +else
>> +gphy = devm_phy_create(dev, child, _ops);
>>  if (IS_ERR(gphy)) {
>>  ret = PTR_ERR(gphy);
>>  goto put_child;
>> @@ -2507,7 +2519,7 @@ static int cdns_torrent_phy_probe(struct
>> platform_device *pdev)
>>  goto put_lnk_rst;
>>  }
>>
>> -if (cdns_phy->nsubnodes > 1) {
>> +if (cdns_phy->nsubnodes > 1 && !already_configured) {
>>  ret = cdns_torrent_phy_configure_multilink(cdns_phy);
>>  if (ret)
>>  goto put_lnk_rst;
>> --
>> 2.17.1
>

RE: [PATCH 3/4] phy: cadence-torrent: Do not configure SERDES if it's already configured

2021-03-18 Thread Swapnil Kashinath Jakhade 



> -Original Message-
> From: Kishon Vijay Abraham I 
> Sent: Wednesday, March 10, 2021 9:25 PM
> To: Kishon Vijay Abraham I ; Vinod Koul
> ; Rob Herring ; Philipp Zabel
> ; Swapnil Kashinath Jakhade
> 
> Cc: linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; Lokesh Vutla
> ; linux-...@lists.infradead.org
> Subject: [PATCH 3/4] phy: cadence-torrent: Do not configure SERDES if it's
> already configured
> 
> EXTERNAL MAIL
> 
> 
> Do not configure torrent SERDES if it's already configured.
> 
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  drivers/phy/cadence/phy-cadence-torrent.c | 32 ---
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/phy/cadence/phy-cadence-torrent.c
> b/drivers/phy/cadence/phy-cadence-torrent.c
> index ab51c4bf7b30..5ee1657f5a1c 100644
> --- a/drivers/phy/cadence/phy-cadence-torrent.c
> +++ b/drivers/phy/cadence/phy-cadence-torrent.c
> @@ -371,6 +371,10 @@ static const struct phy_ops cdns_torrent_phy_ops =
> {
>   .owner  = THIS_MODULE,
>  };
> 
> +static const struct phy_ops noop_ops = {
> + .owner  = THIS_MODULE,
> +};
> +
>  struct cdns_reg_pairs {
>   u32 val;
>   u32 off;
> @@ -2306,6 +2310,7 @@ static int cdns_torrent_phy_probe(struct
> platform_device *pdev)
>   struct device_node *child;
>   int ret, subnodes, node = 0, i;
>   u32 total_num_lanes = 0;
> + int already_configured;
>   u8 init_dp_regmap = 0;
>   u32 phy_type;
> 
> @@ -2344,16 +2349,20 @@ static int cdns_torrent_phy_probe(struct
> platform_device *pdev)
>   if (ret)
>   return ret;
> 
> - ret = cdns_torrent_reset(cdns_phy);
> - if (ret)
> - goto clk_cleanup;
> + regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1,
> _configured);
> 
> - ret = cdns_torrent_clk(cdns_phy);
> - if (ret)
> - goto clk_cleanup;
> + if (!already_configured) {
> + ret = cdns_torrent_reset(cdns_phy);
> + if (ret)
> + goto clk_cleanup;
> +
> + ret = cdns_torrent_clk(cdns_phy);
> + if (ret)
> + goto clk_cleanup;
> 
Should if (!already_configured) be checked while calling 
clk_disable_unprepare()?
Otherwise,
Reviewed-by: Swapnil Jakhade 

Thanks & regards,
Swapnil

> - /* Enable APB */
> - reset_control_deassert(cdns_phy->apb_rst);
> + /* Enable APB */
> + reset_control_deassert(cdns_phy->apb_rst);
> + }
> 
>   for_each_available_child_of_node(dev->of_node, child) {
>   struct phy *gphy;
> @@ -2423,7 +2432,10 @@ static int cdns_torrent_phy_probe(struct
> platform_device *pdev)
>   of_property_read_u32(child, "cdns,ssc-mode",
>_phy->phys[node].ssc_mode);
> 
> - gphy = devm_phy_create(dev, child,
> _torrent_phy_ops);
> + if (!already_configured)
> + gphy = devm_phy_create(dev, child,
> _torrent_phy_ops);
> + else
> + gphy = devm_phy_create(dev, child, _ops);
>   if (IS_ERR(gphy)) {
>   ret = PTR_ERR(gphy);
>   goto put_child;
> @@ -2507,7 +2519,7 @@ static int cdns_torrent_phy_probe(struct
> platform_device *pdev)
>   goto put_lnk_rst;
>   }
> 
> - if (cdns_phy->nsubnodes > 1) {
> + if (cdns_phy->nsubnodes > 1 && !already_configured) {
>   ret = cdns_torrent_phy_configure_multilink(cdns_phy);
>   if (ret)
>   goto put_lnk_rst;
> --
> 2.17.1

[PATCH 3/4] phy: cadence-torrent: Do not configure SERDES if it's already configured

2021-03-10 Thread Kishon Vijay Abraham I 
Do not configure torrent SERDES if it's already configured.

Signed-off-by: Kishon Vijay Abraham I 
---
 drivers/phy/cadence/phy-cadence-torrent.c | 32 ---
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/cadence/phy-cadence-torrent.c 
b/drivers/phy/cadence/phy-cadence-torrent.c
index ab51c4bf7b30..5ee1657f5a1c 100644
--- a/drivers/phy/cadence/phy-cadence-torrent.c
+++ b/drivers/phy/cadence/phy-cadence-torrent.c
@@ -371,6 +371,10 @@ static const struct phy_ops cdns_torrent_phy_ops = {
.owner  = THIS_MODULE,
 };
 
+static const struct phy_ops noop_ops = {
+   .owner  = THIS_MODULE,
+};
+
 struct cdns_reg_pairs {
u32 val;
u32 off;
@@ -2306,6 +2310,7 @@ static int cdns_torrent_phy_probe(struct platform_device 
*pdev)
struct device_node *child;
int ret, subnodes, node = 0, i;
u32 total_num_lanes = 0;
+   int already_configured;
u8 init_dp_regmap = 0;
u32 phy_type;
 
@@ -2344,16 +2349,20 @@ static int cdns_torrent_phy_probe(struct 
platform_device *pdev)
if (ret)
return ret;
 
-   ret = cdns_torrent_reset(cdns_phy);
-   if (ret)
-   goto clk_cleanup;
+   regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1, _configured);
 
-   ret = cdns_torrent_clk(cdns_phy);
-   if (ret)
-   goto clk_cleanup;
+   if (!already_configured) {
+   ret = cdns_torrent_reset(cdns_phy);
+   if (ret)
+   goto clk_cleanup;
+
+   ret = cdns_torrent_clk(cdns_phy);
+   if (ret)
+   goto clk_cleanup;
 
-   /* Enable APB */
-   reset_control_deassert(cdns_phy->apb_rst);
+   /* Enable APB */
+   reset_control_deassert(cdns_phy->apb_rst);
+   }
 
for_each_available_child_of_node(dev->of_node, child) {
struct phy *gphy;
@@ -2423,7 +2432,10 @@ static int cdns_torrent_phy_probe(struct platform_device 
*pdev)
of_property_read_u32(child, "cdns,ssc-mode",
 _phy->phys[node].ssc_mode);
 
-   gphy = devm_phy_create(dev, child, _torrent_phy_ops);
+   if (!already_configured)
+   gphy = devm_phy_create(dev, child, 
_torrent_phy_ops);
+   else
+   gphy = devm_phy_create(dev, child, _ops);
if (IS_ERR(gphy)) {
ret = PTR_ERR(gphy);
goto put_child;
@@ -2507,7 +2519,7 @@ static int cdns_torrent_phy_probe(struct platform_device 
*pdev)
goto put_lnk_rst;
}
 
-   if (cdns_phy->nsubnodes > 1) {
+   if (cdns_phy->nsubnodes > 1 && !already_configured) {
ret = cdns_torrent_phy_configure_multilink(cdns_phy);
if (ret)
goto put_lnk_rst;
-- 
2.17.1