diff mbox series

[net-next,v7,3/9] net: txgbe: Register fixed rate clock

Message ID 20230509022734.148970-4-jiawenwu@trustnetic.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series TXGBE PHYLINK support | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 4 maintainers not CCed: kuba@kernel.org edumazet@google.com davem@davemloft.net pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 78 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiawen Wu May 9, 2023, 2:27 a.m. UTC 
In order for I2C to be able to work in standard mode, register a fixed
rate clock for each I2C device.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |  1 +
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 41 +++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  2 +
 3 files changed, 44 insertions(+)

Comments

Simon Horman May 9, 2023, 3:32 p.m. UTC | #1 
On Tue, May 09, 2023 at 10:27:28AM +0800, Jiawen Wu wrote:
> In order for I2C to be able to work in standard mode, register a fixed
> rate clock for each I2C device.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

...

> @@ -70,6 +72,32 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
>  	return software_node_register_node_group(nodes->group);
>  }
>  
> +static int txgbe_clock_register(struct txgbe *txgbe)
> +{
> +	struct pci_dev *pdev = txgbe->wx->pdev;
> +	struct clk_lookup *clock;
> +	char clk_name[32];
> +	struct clk *clk;
> +
> +	snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
> +		 (pdev->bus->number << 8) | pdev->devfn);
> +
> +	clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	clock = clkdev_create(clk, NULL, clk_name);
> +	if (!clock) {
> +		clk_unregister(clk);
> +		return PTR_ERR(clock);

Hi Jiawen,

Sorry for missing this earlier, but the above error handling doesn't seem
right.

   * This error condition is met if clock == NULL
   * So the above is returning PTR_ERR(NULL), which is a yellow flag to me.
     In any case, PTR_ERR(NULL) => 0 is returned on error.
   * The caller treats a 0 return value as success.

   Perhaps this should be: return -ENOMEM?

> +	}
> +
> +	txgbe->clk = clk;
> +	txgbe->clock = clock;
> +
> +	return 0;
> +}
> +
>  int txgbe_init_phy(struct txgbe *txgbe)
>  {
>  	int ret;
> @@ -80,10 +108,23 @@ int txgbe_init_phy(struct txgbe *txgbe)
>  		return ret;
>  	}
>  
> +	ret = txgbe_clock_register(txgbe);
> +	if (ret) {
> +		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
> +		goto err_unregister_swnode;
> +	}
> +
>  	return 0;

...
Jiawen Wu May 10, 2023, 6:47 a.m. UTC | #2 
On Tuesday, May 9, 2023 11:32 PM, Simon Horman wrote:
> On Tue, May 09, 2023 at 10:27:28AM +0800, Jiawen Wu wrote:
> > In order for I2C to be able to work in standard mode, register a fixed
> > rate clock for each I2C device.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> 
> ...
> 
> > @@ -70,6 +72,32 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
> >  	return software_node_register_node_group(nodes->group);
> >  }
> >
> > +static int txgbe_clock_register(struct txgbe *txgbe) {
> > +	struct pci_dev *pdev = txgbe->wx->pdev;
> > +	struct clk_lookup *clock;
> > +	char clk_name[32];
> > +	struct clk *clk;
> > +
> > +	snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
> > +		 (pdev->bus->number << 8) | pdev->devfn);
> > +
> > +	clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +
> > +	clock = clkdev_create(clk, NULL, clk_name);
> > +	if (!clock) {
> > +		clk_unregister(clk);
> > +		return PTR_ERR(clock);
> 
> Hi Jiawen,
> 
> Sorry for missing this earlier, but the above error handling doesn't seem right.
> 
>    * This error condition is met if clock == NULL
>    * So the above is returning PTR_ERR(NULL), which is a yellow flag to me.
>      In any case, PTR_ERR(NULL) => 0 is returned on error.
>    * The caller treats a 0 return value as success.
> 
>    Perhaps this should be: return -ENOMEM?

No problem, I will fix it in patch v8.

> 
> > +	}
> > +
> > +	txgbe->clk = clk;
> > +	txgbe->clock = clock;
> > +
> > +	return 0;
> > +}
> > +
> >  int txgbe_init_phy(struct txgbe *txgbe)  {
> >  	int ret;
> > @@ -80,10 +108,23 @@ int txgbe_init_phy(struct txgbe *txgbe)
> >  		return ret;
> >  	}
> >
> > +	ret = txgbe_clock_register(txgbe);
> > +	if (ret) {
> > +		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
> > +		goto err_unregister_swnode;
> > +	}
> > +
> >  	return 0;
> 
> ...
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index c9d88673d306..a62614eeed2e 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -40,6 +40,7 @@  config NGBE
 config TXGBE
 	tristate "Wangxun(R) 10GbE PCI Express adapters support"
 	depends on PCI
+	select COMMON_CLK
 	select LIBWX
 	help
 	  This driver supports Wangxun(R) 10GbE PCI Express family of
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 3476074869cb..052354b650a5 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -2,6 +2,8 @@ 
 /* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
 
 #include <linux/gpio/property.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
 #include <linux/i2c.h>
 #include <linux/pci.h>
 
@@ -70,6 +72,32 @@  static int txgbe_swnodes_register(struct txgbe *txgbe)
 	return software_node_register_node_group(nodes->group);
 }
 
+static int txgbe_clock_register(struct txgbe *txgbe)
+{
+	struct pci_dev *pdev = txgbe->wx->pdev;
+	struct clk_lookup *clock;
+	char clk_name[32];
+	struct clk *clk;
+
+	snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
+		 (pdev->bus->number << 8) | pdev->devfn);
+
+	clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	clock = clkdev_create(clk, NULL, clk_name);
+	if (!clock) {
+		clk_unregister(clk);
+		return PTR_ERR(clock);
+	}
+
+	txgbe->clk = clk;
+	txgbe->clock = clock;
+
+	return 0;
+}
+
 int txgbe_init_phy(struct txgbe *txgbe)
 {
 	int ret;
@@ -80,10 +108,23 @@  int txgbe_init_phy(struct txgbe *txgbe)
 		return ret;
 	}
 
+	ret = txgbe_clock_register(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
+		goto err_unregister_swnode;
+	}
+
 	return 0;
+
+err_unregister_swnode:
+	software_node_unregister_node_group(txgbe->nodes.group);
+
+	return ret;
 }
 
 void txgbe_remove_phy(struct txgbe *txgbe)
 {
+	clkdev_drop(txgbe->clock);
+	clk_unregister(txgbe->clk);
 	software_node_unregister_node_group(txgbe->nodes.group);
 }
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 5bef0f9df523..cdbc4b37f832 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -146,6 +146,8 @@  struct txgbe_nodes {
 struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
+	struct clk_lookup *clock;
+	struct clk *clk;
 };
 
 #endif /* _TXGBE_TYPE_H_ */