On 02/06/2023 07:01, Hans Hu wrote: > Add Zhaoxin I2C controller driver. It provides the access to the i2c > busses, which connects to the touchpad, eeprom, etc. > > Zhaoxin I2C controller has two separate busses, so may accommodate up > to two I2C adapters. Those adapters are listed in the ACPI namespace > with the "IIC1D17" HID, and probed by a platform driver. > > The driver works with IRQ mode, and supports basic I2C features. Flags > I2C_AQ_NO_ZERO_LEN and I2C_AQ_COMB_WRITE_THEN_READ are used to limit > the unsupported access. > > Change since v2: > * fixed some code style issues > * check if "timeout == 0" first, then check if event $condition is true > * add warnning message for "timeout == 1" > Link: https://lore.kernel.org/all/20230531110058.n7ubjp2kzlx7tuoc@intel.intel/ > > Change since v1: > * remove some useless include files and sort the rest. > * use mmio bar distinguish host index. > * use pci-dev's name and i2c-dev's name rename adapter's name. > * remove some debug code, fix some code style issue. > Link: https://lore.kernel.org/all/20230504060043.13155-1-hanshu-oc@xxxxxxxxxxx/ > > Signed-off-by: Hans Hu <hanshu-oc@xxxxxxxxxxx> > --- > drivers/i2c/busses/Kconfig | 10 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-zhaoxin.c | 645 +++++++++++++++++++++++++++++++ > 3 files changed, 656 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-zhaoxin.c > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 87600b4aacb3..4c2d302184aa 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -333,6 +333,16 @@ config I2C_VIAPRO > This driver can also be built as a module. If so, the module > will be called i2c-viapro. > > +config I2C_ZHAOXIN > + tristate "ZHAOXIN I2C Interface" > + depends on PCI && ACPI Why this cannot be compile tested? > + help > + If you say yes to this option, support will be included for the > + ZHAOXIN I2C interface > + > + This driver can also be built as a module. If so, the module > + will be called i2c-zhaoxin. > + > if ACPI > > comment "ACPI drivers" > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index af56fe2c75c0..cc470ce470ca 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_SIS630) += i2c-sis630.o > obj-$(CONFIG_I2C_SIS96X) += i2c-sis96x.o > obj-$(CONFIG_I2C_VIA) += i2c-via.o > obj-$(CONFIG_I2C_VIAPRO) += i2c-viapro.o > +obj-$(CONFIG_I2C_ZHAOXIN) += i2c-zhaoxin.o > > # Mac SMBus host controller drivers > obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o > diff --git a/drivers/i2c/busses/i2c-zhaoxin.c b/drivers/i2c/busses/i2c-zhaoxin.c > new file mode 100644 > index 000000000000..61807da9c170 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-zhaoxin.c > @@ -0,0 +1,645 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * i2c-zhaoxin-i2c.c - Zhaoxin I2C controller driver > + * > + * Copyright(c) 2021 Shanghai Zhaoxin Semiconductor Corporation. > + * All rights reserved. > + * > + * CONTACTS: > + * > + * Hans Hu hanshu@xxxxxxxxxxx > + * Cobe Chen cobechen@xxxxxxxxxxx > + */ > + > +#define DRIVER_VERSION "1.4.0" > + Drop. Linux already has a version. > + > +static int zxi2c_parse_resources(struct zxi2c *i2c) > +{ > + struct platform_device *pdev = to_platform_device(i2c->dev); > + struct resource *res; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + i2c->regs = devm_ioremap_resource(&pdev->dev, res); There is a helper for these two. > + if (IS_ERR(i2c->regs)) > + return dev_err_probe(&pdev->dev, PTR_ERR(i2c->regs), > + "get base addr failed\n"); > + > + if (res->start & 0x20) > + i2c->reset_bitmask = BIT(4); > + else > + i2c->reset_bitmask = BIT(5); > + > + i2c->irq = platform_get_irq(pdev, 0); > + if (i2c->irq < 0) > + return dev_err_probe(&pdev->dev, i2c->irq, > + "get irq failed\n"); > + > + zxi2c_get_bus_speed(i2c); > + > + i2c->hrv = get_reversion(i2c->regs); > + > + i2c->dynamic = get_dynamic_clock(i2c->regs); > + set_dynamic_clock(i2c->regs, i2c->dynamic); > + > + i2c->fstp = get_fstp_value(i2c->regs); > + zxi2c_set_bus_speed(i2c); > + > + return 0; > +} > + > +static int zxi2c_probe(struct platform_device *pdev) > +{ > + int error; > + struct zxi2c *i2c; > + struct pci_dev *pci; > + struct device *dev; > + > + /* make sure this is zhaoxin platform */ Why? How can you match to different platform? > + dev = pdev->dev.parent; > + if (dev && dev_is_pci(dev)) { > + pci = to_pci_dev(dev); > + if (pci->vendor != 0x1d17 || pci->device != 0x1001) > + return -ENODEV; > + } else { > + return -ENODEV; > + } > + > + i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL); > + if (IS_ERR(i2c)) { > + return dev_err_probe(&pdev->dev, -ENOMEM, > + "devm_kzalloc failed\n"); Don't print ENOMEM. Run: 1. Checkpatch 2. Coccinelle to fix trivial issues. > + } > + > + i2c->pci = pci; > + i2c->dev = &pdev->dev; > + error = zxi2c_parse_resources(i2c); > + if (error) > + return error; > + > + platform_set_drvdata(pdev, (void *)i2c); > + > + error = devm_request_irq(&pdev->dev, i2c->irq, zxi2c_irq_handle, > + IRQF_SHARED, pdev->name, i2c); > + if (error < 0) > + return dev_err_probe(i2c->dev, error, > + "IRQ%d request failed\n", > + i2c->irq); > + > + init_waitqueue_head(&i2c->waitq); > + > + i2c->adap.owner = THIS_MODULE; > + i2c->adap.algo = &zxi2c_algorithm; > + i2c->adap.retries = 2; > + i2c->adap.quirks = &zxi2c_quirks; > + i2c->adap.dev.parent = &pdev->dev; > + ACPI_COMPANION_SET(&i2c->adap.dev, ACPI_COMPANION(&pdev->dev)); > + snprintf(i2c->adap.name, sizeof(i2c->adap.name), > + "%s-%s", dev_name(&pci->dev), dev_name(i2c->dev)); > + i2c_set_adapdata(&i2c->adap, i2c); > + > + error = i2c_add_adapter(&i2c->adap); > + if (unlikely(error)) > + return dev_err_probe(i2c->dev, error, > + "adapter registration failed\n"); > + > + dev_info(i2c->dev, "adapter /dev/i2c-%d registered. hw version %x\n", > + i2c->adap.nr, i2c->hrv); > + > + return 0; > +} > + > +static int zxi2c_remove(struct platform_device *pdev) > +{ > + struct zxi2c *i2c = platform_get_drvdata(pdev); > + > + i2c_lock_bus(&i2c->adap, I2C_LOCK_ROOT_ADAPTER); > + > + zxi2c_module_reset(i2c); > + master_regs_reset(i2c->regs); > + > + devm_free_irq(&pdev->dev, i2c->irq, i2c); > + > + i2c_unlock_bus(&i2c->adap, I2C_LOCK_ROOT_ADAPTER); > + > + i2c_del_adapter(&i2c->adap); > + > + platform_set_drvdata(pdev, NULL); > + devm_kfree(&pdev->dev, i2c); Why do you do it? Its devm. > + > + dev_info(&pdev->dev, "adapter unregistered\n"); Drop simple probe or function success statuses. Kernel has other ways to get this information. ... > + .acpi_match_table = ACPI_PTR(zxi2c_acpi_match), Drop ACPI_PTR. This will cause warnings. Best regards, Krzysztof
