Re: [PATCH net-next v10 4/9] net: txgbe: Register I2C platform device

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Thu, Jun 01, 2023 at 11:01:35AM +0800, Jiawen Wu wrote:
> Register the platform device to use Designware I2C bus master driver.
> Use regmap to read/write I2C device region from given base offset.

LGTM from I²C device registration point of view,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> Signed-off-by: Jiawen Wu <jiawenwu@xxxxxxxxxxxxxx>
> Reviewed-by: Andrew Lunn <andrew@xxxxxxx>
> Reviewed-by: Piotr Raczynski <piotr.raczynski@xxxxxxxxx>
> ---
>  drivers/net/ethernet/wangxun/Kconfig          |  3 +
>  .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 70 +++++++++++++++++++
>  .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  4 ++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
> index 190d42a203b4..128cc1cb0605 100644
> --- a/drivers/net/ethernet/wangxun/Kconfig
> +++ b/drivers/net/ethernet/wangxun/Kconfig
> @@ -41,6 +41,9 @@ config TXGBE
>  	tristate "Wangxun(R) 10GbE PCI Express adapters support"
>  	depends on PCI
>  	depends on COMMON_CLK
> +	select REGMAP
> +	select I2C
> +	select I2C_DESIGNWARE_PLATFORM
>  	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 06506cfb8d06..24a729150e08 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> @@ -6,6 +6,8 @@
>  #include <linux/clkdev.h>
>  #include <linux/i2c.h>
>  #include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  
>  #include "../libwx/wx_type.h"
>  #include "txgbe_type.h"
> @@ -98,6 +100,64 @@ static int txgbe_clock_register(struct txgbe *txgbe)
>  	return 0;
>  }
>  
> +static int txgbe_i2c_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct wx *wx = context;
> +
> +	*val = rd32(wx, reg + TXGBE_I2C_BASE);
> +
> +	return 0;
> +}
> +
> +static int txgbe_i2c_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct wx *wx = context;
> +
> +	wr32(wx, reg + TXGBE_I2C_BASE, val);
> +
> +	return 0;
> +}
> +
> +static const struct regmap_config i2c_regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_read = txgbe_i2c_read,
> +	.reg_write = txgbe_i2c_write,
> +	.fast_io = true,
> +};
> +
> +static int txgbe_i2c_register(struct txgbe *txgbe)
> +{
> +	struct platform_device_info info = {};
> +	struct platform_device *i2c_dev;
> +	struct regmap *i2c_regmap;
> +	struct pci_dev *pdev;
> +	struct wx *wx;
> +
> +	wx = txgbe->wx;
> +	pdev = wx->pdev;
> +	i2c_regmap = devm_regmap_init(&pdev->dev, NULL, wx, &i2c_regmap_config);
> +	if (IS_ERR(i2c_regmap)) {
> +		wx_err(wx, "failed to init I2C regmap\n");
> +		return PTR_ERR(i2c_regmap);
> +	}
> +
> +	info.parent = &pdev->dev;
> +	info.fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_I2C]);
> +	info.name = "i2c_designware";
> +	info.id = (pdev->bus->number << 8) | pdev->devfn;
> +
> +	info.res = &DEFINE_RES_IRQ(pdev->irq);
> +	info.num_res = 1;
> +	i2c_dev = platform_device_register_full(&info);
> +	if (IS_ERR(i2c_dev))
> +		return PTR_ERR(i2c_dev);
> +
> +	txgbe->i2c_dev = i2c_dev;
> +
> +	return 0;
> +}
> +
>  int txgbe_init_phy(struct txgbe *txgbe)
>  {
>  	int ret;
> @@ -114,8 +174,17 @@ int txgbe_init_phy(struct txgbe *txgbe)
>  		goto err_unregister_swnode;
>  	}
>  
> +	ret = txgbe_i2c_register(txgbe);
> +	if (ret) {
> +		wx_err(txgbe->wx, "failed to init i2c interface: %d\n", ret);
> +		goto err_unregister_clk;
> +	}
> +
>  	return 0;
>  
> +err_unregister_clk:
> +	clkdev_drop(txgbe->clock);
> +	clk_unregister(txgbe->clk);
>  err_unregister_swnode:
>  	software_node_unregister_node_group(txgbe->nodes.group);
>  
> @@ -124,6 +193,7 @@ int txgbe_init_phy(struct txgbe *txgbe)
>  
>  void txgbe_remove_phy(struct txgbe *txgbe)
>  {
> +	platform_device_unregister(txgbe->i2c_dev);
>  	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 cdbc4b37f832..55979abf01f2 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> @@ -55,6 +55,9 @@
>  #define TXGBE_TS_CTL                            0x10300
>  #define TXGBE_TS_CTL_EVAL_MD                    BIT(31)
>  
> +/* I2C registers */
> +#define TXGBE_I2C_BASE                          0x14900
> +
>  /* Part Number String Length */
>  #define TXGBE_PBANUM_LENGTH                     32
>  
> @@ -146,6 +149,7 @@ struct txgbe_nodes {
>  struct txgbe {
>  	struct wx *wx;
>  	struct txgbe_nodes nodes;
> +	struct platform_device *i2c_dev;
>  	struct clk_lookup *clock;
>  	struct clk *clk;
>  };
> -- 
> 2.27.0
> 

-- 
With Best Regards,
Andy Shevchenko