| Message ID | 20250424184952.1290019-1-pratap.nirujogi@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] i2c: amd-isp: Add ISP i2c-designware driver | expand |
Hi Pratap, On Thu, Apr 24, 2025 at 02:49:26PM -0400, Pratap Nirujogi wrote: > The camera sensor is connected via ISP I2C bus in AMD SOC > architectures. Add new I2C designware driver to support > new camera sensors on AMD HW. > > Co-developed-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> > Signed-off-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> > Co-developed-by: Bin Du <bin.du@amd.com> > Signed-off-by: Bin Du <bin.du@amd.com> > Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> merged to i2c/i2c-host. Thanks, Andi
Hi Andi, Happy to see this patch is accepted and merged. Thanks very much! Thanks, Pratap On 4/28/2025 3:30 AM, Andi Shyti wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Hi Pratap, > > On Thu, Apr 24, 2025 at 02:49:26PM -0400, Pratap Nirujogi wrote: >> The camera sensor is connected via ISP I2C bus in AMD SOC >> architectures. Add new I2C designware driver to support >> new camera sensors on AMD HW. >> >> Co-developed-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> >> Signed-off-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> >> Co-developed-by: Bin Du <bin.du@amd.com> >> Signed-off-by: Bin Du <bin.du@amd.com> >> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> > > merged to i2c/i2c-host. > > Thanks, > Andi
Le 24/04/2025 à 20:49, Pratap Nirujogi a écrit : > The camera sensor is connected via ISP I2C bus in AMD SOC > architectures. Add new I2C designware driver to support > new camera sensors on AMD HW. > > Co-developed-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> > Signed-off-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> > Co-developed-by: Bin Du <bin.du@amd.com> > Signed-off-by: Bin Du <bin.du@amd.com> > Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> > --- > Changes v1 -> v2: > > * Remove dependency on exported symbol "isp_power_set()". Use pm_runtime ops to power on/off ISP controller. > * Remove hardcoding adapter id to 99. Instead switched to use dynamically allocated adapter id. > * Cleanup header files. > * Replace subsys_initcall() with default module_init() > * Update copyright header and license info. > * Update MAINTAINERS details for i2c-designware-amdisp.c > * Fix coding errors based on review feedback. > > MAINTAINERS | 7 + > drivers/i2c/busses/Kconfig | 10 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-designware-amdisp.c | 205 +++++++++++++++++++++ > 4 files changed, 223 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index f31aeb6b452e..65b6d985e1ed 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -23519,6 +23519,13 @@ L: linux-i2c@vger.kernel.org > S: Supported > F: drivers/i2c/busses/i2c-designware-* > > +SYNOPSYS DESIGNWARE I2C DRIVER - AMDISP > +M: Nirujogi Pratap <pratap.nirujogi@amd.com> > +M: Bin Du <bin.du@amd.com> > +L: linux-i2c@vger.kernel.org > +S: Maintained > +F: drivers/i2c/busses/i2c-designware-amdisp.c > + > SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER > M: Jaehoon Chung <jh80.chung@samsung.com> > L: linux-mmc@vger.kernel.org > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 83c88c79afe2..adb2910525b1 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -592,6 +592,16 @@ config I2C_DESIGNWARE_PLATFORM > This driver can also be built as a module. If so, the module > will be called i2c-designware-platform. > > +config I2C_DESIGNWARE_AMDISP > + tristate "Synopsys DesignWare Platform for AMDISP" > + depends on I2C_DESIGNWARE_CORE > + help > + If you say yes to this option, support will be included for the > + AMDISP Synopsys DesignWare I2C adapter. > + > + This driver can also be built as a module. If so, the module > + will be called amd_isp_i2c_designware. > + > config I2C_DESIGNWARE_AMDPSP > bool "AMD PSP I2C semaphore support" > depends on ACPI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index c1252e2b779e..04db855fdfd6 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -58,6 +58,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o > i2c-designware-platform-y := i2c-designware-platdrv.o > i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_AMDPSP) += i2c-designware-amdpsp.o > i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c-designware-baytrail.o > +obj-$(CONFIG_I2C_DESIGNWARE_AMDISP) += i2c-designware-amdisp.o > obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o > i2c-designware-pci-y := i2c-designware-pcidrv.o > obj-$(CONFIG_I2C_DIGICOLOR) += i2c-digicolor.o > diff --git a/drivers/i2c/busses/i2c-designware-amdisp.c b/drivers/i2c/busses/i2c-designware-amdisp.c > new file mode 100644 > index 000000000000..ad6f08338124 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-designware-amdisp.c > @@ -0,0 +1,205 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Based on Synopsys DesignWare I2C adapter driver. > + * > + * Copyright (C) 2025 Advanced Micro Devices, Inc. > + */ > + > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > + > +#include "i2c-designware-core.h" > + > +#define DRV_NAME "amd_isp_i2c_designware" > +#define AMD_ISP_I2C_INPUT_CLK 100 /* Mhz */ > + > +static void amd_isp_dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *i2c_dev) > +{ > + pm_runtime_disable(i2c_dev->dev); > + > + if (i2c_dev->shared_with_punit) > + pm_runtime_put_noidle(i2c_dev->dev); > +} > + > +static inline u32 amd_isp_dw_i2c_get_clk_rate(struct dw_i2c_dev *i2c_dev) > +{ > + return AMD_ISP_I2C_INPUT_CLK * 1000; > +} > + > +static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev) > +{ > + struct dw_i2c_dev *isp_i2c_dev; > + struct i2c_adapter *adap; > + int ret; > + > + isp_i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*isp_i2c_dev), GFP_KERNEL); > + if (!isp_i2c_dev) > + return -ENOMEM; > + isp_i2c_dev->dev = &pdev->dev; > + > + pdev->dev.init_name = DRV_NAME; > + > + /* > + * Use the polling mode to send/receive the data, because > + * no IRQ connection from ISP I2C > + */ > + isp_i2c_dev->flags |= ACCESS_POLLING; > + platform_set_drvdata(pdev, isp_i2c_dev); > + > + isp_i2c_dev->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(isp_i2c_dev->base)) > + return dev_err_probe(&pdev->dev, PTR_ERR(isp_i2c_dev->base), > + "failed to get IOMEM resource\n"); > + > + isp_i2c_dev->get_clk_rate_khz = amd_isp_dw_i2c_get_clk_rate; > + ret = i2c_dw_fw_parse_and_configure(isp_i2c_dev); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "failed to parse i2c dw fwnode and configure\n"); > + > + i2c_dw_configure(isp_i2c_dev); > + > + adap = &isp_i2c_dev->adapter; > + adap->owner = THIS_MODULE; > + ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); > + adap->dev.of_node = pdev->dev.of_node; > + /* use dynamically allocated adapter id */ > + adap->nr = -1; > + > + if (isp_i2c_dev->flags & ACCESS_NO_IRQ_SUSPEND) > + dev_pm_set_driver_flags(&pdev->dev, > + DPM_FLAG_SMART_PREPARE); > + else > + dev_pm_set_driver_flags(&pdev->dev, > + DPM_FLAG_SMART_PREPARE | > + DPM_FLAG_SMART_SUSPEND); > + > + device_enable_async_suspend(&pdev->dev); > + > + if (isp_i2c_dev->shared_with_punit) > + pm_runtime_get_noresume(&pdev->dev); > + > + pm_runtime_enable(&pdev->dev); > + pm_runtime_get_sync(&pdev->dev); > + > + ret = i2c_dw_probe(isp_i2c_dev); > + if (ret) { > + dev_err_probe(&pdev->dev, ret, "i2c_dw_probe failed\n"); > + goto error_release_rpm; > + } > + > + pm_runtime_put_sync(&pdev->dev); > + > + return 0; > + > +error_release_rpm: > + amd_isp_dw_i2c_plat_pm_cleanup(isp_i2c_dev); > + pm_runtime_put_sync(&pdev->dev); > + return ret; > +} > + > +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) > +{ > + struct dw_i2c_dev *isp_i2c_dev = platform_get_drvdata(pdev); > + > + pm_runtime_get_sync(&pdev->dev); > + > + i2c_del_adapter(&isp_i2c_dev->adapter); Usually, this match a corresponding i2c_add_adapter(). For my own understaning, in which function/calls path is it hidden? Is it needed here? CJ > + > + i2c_dw_disable(isp_i2c_dev); > + > + pm_runtime_put_sync(&pdev->dev); > + amd_isp_dw_i2c_plat_pm_cleanup(isp_i2c_dev); > +} > + > +static int amd_isp_dw_i2c_plat_prepare(struct device *dev) > +{ > + /* > + * If the ACPI companion device object is present for this device, it > + * may be accessed during suspend and resume of other devices via I2C > + * operation regions, so tell the PM core and middle layers to avoid > + * skipping system suspend/resume callbacks for it in that case. > + */ > + return !has_acpi_companion(dev); > +} > + > +static int amd_isp_dw_i2c_plat_runtime_suspend(struct device *dev) > +{ > + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); > + > + if (i_dev->shared_with_punit) > + return 0; > + > + i2c_dw_disable(i_dev); > + i2c_dw_prepare_clk(i_dev, false); > + > + return 0; > +} > + > +static int amd_isp_dw_i2c_plat_suspend(struct device *dev) > +{ > + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); > + int ret; > + > + if (!i_dev) > + return -ENODEV; > + > + ret = amd_isp_dw_i2c_plat_runtime_suspend(dev); > + if (!ret) > + i2c_mark_adapter_suspended(&i_dev->adapter); > + > + return ret; > +} > + > +static int amd_isp_dw_i2c_plat_runtime_resume(struct device *dev) > +{ > + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); > + > + if (!i_dev) > + return -ENODEV; > + > + if (!i_dev->shared_with_punit) > + i2c_dw_prepare_clk(i_dev, true); > + if (i_dev->init) > + i_dev->init(i_dev); > + > + return 0; > +} > + > +static int amd_isp_dw_i2c_plat_resume(struct device *dev) > +{ > + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); > + > + amd_isp_dw_i2c_plat_runtime_resume(dev); > + i2c_mark_adapter_resumed(&i_dev->adapter); > + > + return 0; > +} > + > +static const struct dev_pm_ops amd_isp_dw_i2c_dev_pm_ops = { > + .prepare = pm_sleep_ptr(amd_isp_dw_i2c_plat_prepare), > + LATE_SYSTEM_SLEEP_PM_OPS(amd_isp_dw_i2c_plat_suspend, amd_isp_dw_i2c_plat_resume) > + RUNTIME_PM_OPS(amd_isp_dw_i2c_plat_runtime_suspend, amd_isp_dw_i2c_plat_runtime_resume, NULL) > +}; > + > +/* Work with hotplug and coldplug */ > +MODULE_ALIAS("platform:amd_isp_i2c_designware"); > + > +static struct platform_driver amd_isp_dw_i2c_driver = { > + .probe = amd_isp_dw_i2c_plat_probe, > + .remove = amd_isp_dw_i2c_plat_remove, > + .driver = { > + .name = DRV_NAME, > + .pm = pm_ptr(&amd_isp_dw_i2c_dev_pm_ops), > + }, > +}; > +module_platform_driver(amd_isp_dw_i2c_driver); > + > +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter in AMD ISP"); > +MODULE_IMPORT_NS("I2C_DW"); > +MODULE_IMPORT_NS("I2C_DW_COMMON"); > +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>"); > +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>"); > +MODULE_AUTHOR("Bin Du <bin.du@amd.com>"); > +MODULE_LICENSE("GPL");
Hi CJ, On 5/9/2025 3:11 AM, Christophe JAILLET wrote: > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > Le 24/04/2025 à 20:49, Pratap Nirujogi a écrit : >> The camera sensor is connected via ISP I2C bus in AMD SOC >> architectures. Add new I2C designware driver to support >> new camera sensors on AMD HW. >> >> Co-developed-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> >> Signed-off-by: Venkata Narendra Kumar Gutta <vengutta@amd.com> >> Co-developed-by: Bin Du <bin.du@amd.com> >> Signed-off-by: Bin Du <bin.du@amd.com> >> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> >> --- >> Changes v1 -> v2: >> >> * Remove dependency on exported symbol "isp_power_set()". Use >> pm_runtime ops to power on/off ISP controller. >> * Remove hardcoding adapter id to 99. Instead switched to use >> dynamically allocated adapter id. >> * Cleanup header files. >> * Replace subsys_initcall() with default module_init() >> * Update copyright header and license info. >> * Update MAINTAINERS details for i2c-designware-amdisp.c >> * Fix coding errors based on review feedback. >> >> MAINTAINERS | 7 + >> drivers/i2c/busses/Kconfig | 10 + >> drivers/i2c/busses/Makefile | 1 + >> drivers/i2c/busses/i2c-designware-amdisp.c | 205 +++++++++++++++++++++ >> 4 files changed, 223 insertions(+) >> create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index f31aeb6b452e..65b6d985e1ed 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -23519,6 +23519,13 @@ L: linux-i2c@vger.kernel.org >> S: Supported >> F: drivers/i2c/busses/i2c-designware-* >> >> +SYNOPSYS DESIGNWARE I2C DRIVER - AMDISP >> +M: Nirujogi Pratap <pratap.nirujogi@amd.com> >> +M: Bin Du <bin.du@amd.com> >> +L: linux-i2c@vger.kernel.org >> +S: Maintained >> +F: drivers/i2c/busses/i2c-designware-amdisp.c >> + >> SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER >> M: Jaehoon Chung <jh80.chung@samsung.com> >> L: linux-mmc@vger.kernel.org >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >> index 83c88c79afe2..adb2910525b1 100644 >> --- a/drivers/i2c/busses/Kconfig >> +++ b/drivers/i2c/busses/Kconfig >> @@ -592,6 +592,16 @@ config I2C_DESIGNWARE_PLATFORM >> This driver can also be built as a module. If so, the module >> will be called i2c-designware-platform. >> >> +config I2C_DESIGNWARE_AMDISP >> + tristate "Synopsys DesignWare Platform for AMDISP" >> + depends on I2C_DESIGNWARE_CORE >> + help >> + If you say yes to this option, support will be included for the >> + AMDISP Synopsys DesignWare I2C adapter. >> + >> + This driver can also be built as a module. If so, the module >> + will be called amd_isp_i2c_designware. >> + >> config I2C_DESIGNWARE_AMDPSP >> bool "AMD PSP I2C semaphore support" >> depends on ACPI >> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile >> index c1252e2b779e..04db855fdfd6 100644 >> --- a/drivers/i2c/busses/Makefile >> +++ b/drivers/i2c/busses/Makefile >> @@ -58,6 +58,7 @@ obj- >> $(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c- >> designware-platform.o >> i2c-designware-platform-y := i2c- >> designware-platdrv.o >> i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_AMDPSP) += i2c- >> designware-amdpsp.o >> i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c- >> designware-baytrail.o >> +obj-$(CONFIG_I2C_DESIGNWARE_AMDISP) += i2c-designware-amdisp.o >> obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c- >> designware-pci.o >> i2c-designware-pci-y := i2c- >> designware-pcidrv.o >> obj-$(CONFIG_I2C_DIGICOLOR) += i2c-digicolor.o >> diff --git a/drivers/i2c/busses/i2c-designware-amdisp.c b/drivers/i2c/ >> busses/i2c-designware-amdisp.c >> new file mode 100644 >> index 000000000000..ad6f08338124 >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-designware-amdisp.c >> @@ -0,0 +1,205 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Based on Synopsys DesignWare I2C adapter driver. >> + * >> + * Copyright (C) 2025 Advanced Micro Devices, Inc. >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm_runtime.h> >> + >> +#include "i2c-designware-core.h" >> + >> +#define DRV_NAME "amd_isp_i2c_designware" >> +#define AMD_ISP_I2C_INPUT_CLK 100 /* Mhz */ >> + >> +static void amd_isp_dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *i2c_dev) >> +{ >> + pm_runtime_disable(i2c_dev->dev); >> + >> + if (i2c_dev->shared_with_punit) >> + pm_runtime_put_noidle(i2c_dev->dev); >> +} >> + >> +static inline u32 amd_isp_dw_i2c_get_clk_rate(struct dw_i2c_dev >> *i2c_dev) >> +{ >> + return AMD_ISP_I2C_INPUT_CLK * 1000; >> +} >> + >> +static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev) >> +{ >> + struct dw_i2c_dev *isp_i2c_dev; >> + struct i2c_adapter *adap; >> + int ret; >> + >> + isp_i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*isp_i2c_dev), >> GFP_KERNEL); >> + if (!isp_i2c_dev) >> + return -ENOMEM; >> + isp_i2c_dev->dev = &pdev->dev; >> + >> + pdev->dev.init_name = DRV_NAME; >> + >> + /* >> + * Use the polling mode to send/receive the data, because >> + * no IRQ connection from ISP I2C >> + */ >> + isp_i2c_dev->flags |= ACCESS_POLLING; >> + platform_set_drvdata(pdev, isp_i2c_dev); >> + >> + isp_i2c_dev->base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(isp_i2c_dev->base)) >> + return dev_err_probe(&pdev->dev, PTR_ERR(isp_i2c_dev- >> >base), >> + "failed to get IOMEM resource\n"); >> + >> + isp_i2c_dev->get_clk_rate_khz = amd_isp_dw_i2c_get_clk_rate; >> + ret = i2c_dw_fw_parse_and_configure(isp_i2c_dev); >> + if (ret) >> + return dev_err_probe(&pdev->dev, ret, >> + "failed to parse i2c dw fwnode and >> configure\n"); >> + >> + i2c_dw_configure(isp_i2c_dev); >> + >> + adap = &isp_i2c_dev->adapter; >> + adap->owner = THIS_MODULE; >> + ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); >> + adap->dev.of_node = pdev->dev.of_node; >> + /* use dynamically allocated adapter id */ >> + adap->nr = -1; >> + >> + if (isp_i2c_dev->flags & ACCESS_NO_IRQ_SUSPEND) >> + dev_pm_set_driver_flags(&pdev->dev, >> + DPM_FLAG_SMART_PREPARE); >> + else >> + dev_pm_set_driver_flags(&pdev->dev, >> + DPM_FLAG_SMART_PREPARE | >> + DPM_FLAG_SMART_SUSPEND); >> + >> + device_enable_async_suspend(&pdev->dev); >> + >> + if (isp_i2c_dev->shared_with_punit) >> + pm_runtime_get_noresume(&pdev->dev); >> + >> + pm_runtime_enable(&pdev->dev); >> + pm_runtime_get_sync(&pdev->dev); >> + >> + ret = i2c_dw_probe(isp_i2c_dev); >> + if (ret) { >> + dev_err_probe(&pdev->dev, ret, "i2c_dw_probe failed\n"); >> + goto error_release_rpm; >> + } >> + >> + pm_runtime_put_sync(&pdev->dev); >> + >> + return 0; >> + >> +error_release_rpm: >> + amd_isp_dw_i2c_plat_pm_cleanup(isp_i2c_dev); >> + pm_runtime_put_sync(&pdev->dev); >> + return ret; >> +} >> + >> +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) >> +{ >> + struct dw_i2c_dev *isp_i2c_dev = platform_get_drvdata(pdev); >> + >> + pm_runtime_get_sync(&pdev->dev); >> + >> + i2c_del_adapter(&isp_i2c_dev->adapter); > > Usually, this match a corresponding i2c_add_adapter(). > > For my own understaning, in which function/calls path is it hidden? > Is it needed here? > > CJ > i2c_add_adapter() in this case gets called in probe. Please refer the below call sequence for details. amd_isp_dw_i2c_plat_probe()-> i2c_dw_probe()-> i2c_dw_probe_master()-> i2c_add_numbered_adapter()-> i2c_add_adapter() Thanks, Pratap > >> + >> + i2c_dw_disable(isp_i2c_dev); >> + >> + pm_runtime_put_sync(&pdev->dev); >> + amd_isp_dw_i2c_plat_pm_cleanup(isp_i2c_dev); >> +} >> + >> +static int amd_isp_dw_i2c_plat_prepare(struct device *dev) >> +{ >> + /* >> + * If the ACPI companion device object is present for this >> device, it >> + * may be accessed during suspend and resume of other devices >> via I2C >> + * operation regions, so tell the PM core and middle layers to >> avoid >> + * skipping system suspend/resume callbacks for it in that case. >> + */ >> + return !has_acpi_companion(dev); >> +} >> + >> +static int amd_isp_dw_i2c_plat_runtime_suspend(struct device *dev) >> +{ >> + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); >> + >> + if (i_dev->shared_with_punit) >> + return 0; >> + >> + i2c_dw_disable(i_dev); >> + i2c_dw_prepare_clk(i_dev, false); >> + >> + return 0; >> +} >> + >> +static int amd_isp_dw_i2c_plat_suspend(struct device *dev) >> +{ >> + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); >> + int ret; >> + >> + if (!i_dev) >> + return -ENODEV; >> + >> + ret = amd_isp_dw_i2c_plat_runtime_suspend(dev); >> + if (!ret) >> + i2c_mark_adapter_suspended(&i_dev->adapter); >> + >> + return ret; >> +} >> + >> +static int amd_isp_dw_i2c_plat_runtime_resume(struct device *dev) >> +{ >> + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); >> + >> + if (!i_dev) >> + return -ENODEV; >> + >> + if (!i_dev->shared_with_punit) >> + i2c_dw_prepare_clk(i_dev, true); >> + if (i_dev->init) >> + i_dev->init(i_dev); >> + >> + return 0; >> +} >> + >> +static int amd_isp_dw_i2c_plat_resume(struct device *dev) >> +{ >> + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); >> + >> + amd_isp_dw_i2c_plat_runtime_resume(dev); >> + i2c_mark_adapter_resumed(&i_dev->adapter); >> + >> + return 0; >> +} >> + >> +static const struct dev_pm_ops amd_isp_dw_i2c_dev_pm_ops = { >> + .prepare = pm_sleep_ptr(amd_isp_dw_i2c_plat_prepare), >> + LATE_SYSTEM_SLEEP_PM_OPS(amd_isp_dw_i2c_plat_suspend, >> amd_isp_dw_i2c_plat_resume) >> + RUNTIME_PM_OPS(amd_isp_dw_i2c_plat_runtime_suspend, >> amd_isp_dw_i2c_plat_runtime_resume, NULL) >> +}; >> + >> +/* Work with hotplug and coldplug */ >> +MODULE_ALIAS("platform:amd_isp_i2c_designware"); >> + >> +static struct platform_driver amd_isp_dw_i2c_driver = { >> + .probe = amd_isp_dw_i2c_plat_probe, >> + .remove = amd_isp_dw_i2c_plat_remove, >> + .driver = { >> + .name = DRV_NAME, >> + .pm = pm_ptr(&amd_isp_dw_i2c_dev_pm_ops), >> + }, >> +}; >> +module_platform_driver(amd_isp_dw_i2c_driver); >> + >> +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter in AMD ISP"); >> +MODULE_IMPORT_NS("I2C_DW"); >> +MODULE_IMPORT_NS("I2C_DW_COMMON"); >> +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>"); >> +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>"); >> +MODULE_AUTHOR("Bin Du <bin.du@amd.com>"); >> +MODULE_LICENSE("GPL"); >
Le 13/05/2025 à 00:24, Nirujogi, Pratap a écrit : > Hi CJ, > > On 5/9/2025 3:11 AM, Christophe JAILLET wrote: >> >> Le 24/04/2025 à 20:49, Pratap Nirujogi a écrit : >>> The camera sensor is connected via ISP I2C bus in AMD SOC >>> architectures. Add new I2C designware driver to support >>> new camera sensors on AMD HW. ... >>> +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) >>> +{ >>> + struct dw_i2c_dev *isp_i2c_dev = platform_get_drvdata(pdev); >>> + >>> + pm_runtime_get_sync(&pdev->dev); >>> + >>> + i2c_del_adapter(&isp_i2c_dev->adapter); >> >> Usually, this match a corresponding i2c_add_adapter(). >> >> For my own understaning, in which function/calls path is it hidden? >> Is it needed here? >> >> CJ >> > i2c_add_adapter() in this case gets called in probe. Please refer the > below call sequence for details. > > amd_isp_dw_i2c_plat_probe()-> i2c_dw_probe()-> i2c_dw_probe_master()-> > i2c_add_numbered_adapter()-> i2c_add_adapter() > > Thanks, > Pratap > Thanks for your feed-back. Maybe having a i2c_dw_remove() which undoes i2c_dw_probe() could help? or, if feasable, having i2c_del_adapter() be called by a devm_add_action_or_reset()? (note that i2c_dw_probe_master() already uses a devm_ function) CJ
On 5/13/2025 1:23 PM, Christophe JAILLET wrote: > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > Le 13/05/2025 à 00:24, Nirujogi, Pratap a écrit : >> Hi CJ, >> >> On 5/9/2025 3:11 AM, Christophe JAILLET wrote: >>> >>> Le 24/04/2025 à 20:49, Pratap Nirujogi a écrit : >>>> The camera sensor is connected via ISP I2C bus in AMD SOC >>>> architectures. Add new I2C designware driver to support >>>> new camera sensors on AMD HW. > > ... > >>>> +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) >>>> +{ >>>> + struct dw_i2c_dev *isp_i2c_dev = platform_get_drvdata(pdev); >>>> + >>>> + pm_runtime_get_sync(&pdev->dev); >>>> + >>>> + i2c_del_adapter(&isp_i2c_dev->adapter); >>> >>> Usually, this match a corresponding i2c_add_adapter(). >>> >>> For my own understaning, in which function/calls path is it hidden? >>> Is it needed here? >>> >>> CJ >>> >> i2c_add_adapter() in this case gets called in probe. Please refer the >> below call sequence for details. >> >> amd_isp_dw_i2c_plat_probe()-> i2c_dw_probe()-> i2c_dw_probe_master()-> >> i2c_add_numbered_adapter()-> i2c_add_adapter() >> >> Thanks, >> Pratap >> > > Thanks for your feed-back. > > Maybe having a i2c_dw_remove() which undoes i2c_dw_probe() could help? > > or, if feasable, having i2c_del_adapter() be called by a > devm_add_action_or_reset()? (note that i2c_dw_probe_master() already > uses a devm_ function) > > CJ > Hi CJ, good suggestions, I agree either adding new i2c_dw_remove() or adding devm_add_action_or_reset(i2c_del_adapter) in i2c_dw_probe_master() would simplify the code and avoid the explicit i2c_del_adapter() call in amd_isp_dw_i2c_plat_remove(). Thanks, Pratap
diff --git a/MAINTAINERS b/MAINTAINERS index f31aeb6b452e..65b6d985e1ed 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -23519,6 +23519,13 @@ L: linux-i2c@vger.kernel.org S: Supported F: drivers/i2c/busses/i2c-designware-* +SYNOPSYS DESIGNWARE I2C DRIVER - AMDISP +M: Nirujogi Pratap <pratap.nirujogi@amd.com> +M: Bin Du <bin.du@amd.com> +L: linux-i2c@vger.kernel.org +S: Maintained +F: drivers/i2c/busses/i2c-designware-amdisp.c + SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER M: Jaehoon Chung <jh80.chung@samsung.com> L: linux-mmc@vger.kernel.org diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 83c88c79afe2..adb2910525b1 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -592,6 +592,16 @@ config I2C_DESIGNWARE_PLATFORM This driver can also be built as a module. If so, the module will be called i2c-designware-platform. +config I2C_DESIGNWARE_AMDISP + tristate "Synopsys DesignWare Platform for AMDISP" + depends on I2C_DESIGNWARE_CORE + help + If you say yes to this option, support will be included for the + AMDISP Synopsys DesignWare I2C adapter. + + This driver can also be built as a module. If so, the module + will be called amd_isp_i2c_designware. + config I2C_DESIGNWARE_AMDPSP bool "AMD PSP I2C semaphore support" depends on ACPI diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index c1252e2b779e..04db855fdfd6 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -58,6 +58,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o i2c-designware-platform-y := i2c-designware-platdrv.o i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_AMDPSP) += i2c-designware-amdpsp.o i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c-designware-baytrail.o +obj-$(CONFIG_I2C_DESIGNWARE_AMDISP) += i2c-designware-amdisp.o obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o i2c-designware-pci-y := i2c-designware-pcidrv.o obj-$(CONFIG_I2C_DIGICOLOR) += i2c-digicolor.o diff --git a/drivers/i2c/busses/i2c-designware-amdisp.c b/drivers/i2c/busses/i2c-designware-amdisp.c new file mode 100644 index 000000000000..ad6f08338124 --- /dev/null +++ b/drivers/i2c/busses/i2c-designware-amdisp.c @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Based on Synopsys DesignWare I2C adapter driver. + * + * Copyright (C) 2025 Advanced Micro Devices, Inc. + */ + +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> + +#include "i2c-designware-core.h" + +#define DRV_NAME "amd_isp_i2c_designware" +#define AMD_ISP_I2C_INPUT_CLK 100 /* Mhz */ + +static void amd_isp_dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *i2c_dev) +{ + pm_runtime_disable(i2c_dev->dev); + + if (i2c_dev->shared_with_punit) + pm_runtime_put_noidle(i2c_dev->dev); +} + +static inline u32 amd_isp_dw_i2c_get_clk_rate(struct dw_i2c_dev *i2c_dev) +{ + return AMD_ISP_I2C_INPUT_CLK * 1000; +} + +static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev) +{ + struct dw_i2c_dev *isp_i2c_dev; + struct i2c_adapter *adap; + int ret; + + isp_i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*isp_i2c_dev), GFP_KERNEL); + if (!isp_i2c_dev) + return -ENOMEM; + isp_i2c_dev->dev = &pdev->dev; + + pdev->dev.init_name = DRV_NAME; + + /* + * Use the polling mode to send/receive the data, because + * no IRQ connection from ISP I2C + */ + isp_i2c_dev->flags |= ACCESS_POLLING; + platform_set_drvdata(pdev, isp_i2c_dev); + + isp_i2c_dev->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(isp_i2c_dev->base)) + return dev_err_probe(&pdev->dev, PTR_ERR(isp_i2c_dev->base), + "failed to get IOMEM resource\n"); + + isp_i2c_dev->get_clk_rate_khz = amd_isp_dw_i2c_get_clk_rate; + ret = i2c_dw_fw_parse_and_configure(isp_i2c_dev); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "failed to parse i2c dw fwnode and configure\n"); + + i2c_dw_configure(isp_i2c_dev); + + adap = &isp_i2c_dev->adapter; + adap->owner = THIS_MODULE; + ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); + adap->dev.of_node = pdev->dev.of_node; + /* use dynamically allocated adapter id */ + adap->nr = -1; + + if (isp_i2c_dev->flags & ACCESS_NO_IRQ_SUSPEND) + dev_pm_set_driver_flags(&pdev->dev, + DPM_FLAG_SMART_PREPARE); + else + dev_pm_set_driver_flags(&pdev->dev, + DPM_FLAG_SMART_PREPARE | + DPM_FLAG_SMART_SUSPEND); + + device_enable_async_suspend(&pdev->dev); + + if (isp_i2c_dev->shared_with_punit) + pm_runtime_get_noresume(&pdev->dev); + + pm_runtime_enable(&pdev->dev); + pm_runtime_get_sync(&pdev->dev); + + ret = i2c_dw_probe(isp_i2c_dev); + if (ret) { + dev_err_probe(&pdev->dev, ret, "i2c_dw_probe failed\n"); + goto error_release_rpm; + } + + pm_runtime_put_sync(&pdev->dev); + + return 0; + +error_release_rpm: + amd_isp_dw_i2c_plat_pm_cleanup(isp_i2c_dev); + pm_runtime_put_sync(&pdev->dev); + return ret; +} + +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) +{ + struct dw_i2c_dev *isp_i2c_dev = platform_get_drvdata(pdev); + + pm_runtime_get_sync(&pdev->dev); + + i2c_del_adapter(&isp_i2c_dev->adapter); + + i2c_dw_disable(isp_i2c_dev); + + pm_runtime_put_sync(&pdev->dev); + amd_isp_dw_i2c_plat_pm_cleanup(isp_i2c_dev); +} + +static int amd_isp_dw_i2c_plat_prepare(struct device *dev) +{ + /* + * If the ACPI companion device object is present for this device, it + * may be accessed during suspend and resume of other devices via I2C + * operation regions, so tell the PM core and middle layers to avoid + * skipping system suspend/resume callbacks for it in that case. + */ + return !has_acpi_companion(dev); +} + +static int amd_isp_dw_i2c_plat_runtime_suspend(struct device *dev) +{ + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); + + if (i_dev->shared_with_punit) + return 0; + + i2c_dw_disable(i_dev); + i2c_dw_prepare_clk(i_dev, false); + + return 0; +} + +static int amd_isp_dw_i2c_plat_suspend(struct device *dev) +{ + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); + int ret; + + if (!i_dev) + return -ENODEV; + + ret = amd_isp_dw_i2c_plat_runtime_suspend(dev); + if (!ret) + i2c_mark_adapter_suspended(&i_dev->adapter); + + return ret; +} + +static int amd_isp_dw_i2c_plat_runtime_resume(struct device *dev) +{ + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); + + if (!i_dev) + return -ENODEV; + + if (!i_dev->shared_with_punit) + i2c_dw_prepare_clk(i_dev, true); + if (i_dev->init) + i_dev->init(i_dev); + + return 0; +} + +static int amd_isp_dw_i2c_plat_resume(struct device *dev) +{ + struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); + + amd_isp_dw_i2c_plat_runtime_resume(dev); + i2c_mark_adapter_resumed(&i_dev->adapter); + + return 0; +} + +static const struct dev_pm_ops amd_isp_dw_i2c_dev_pm_ops = { + .prepare = pm_sleep_ptr(amd_isp_dw_i2c_plat_prepare), + LATE_SYSTEM_SLEEP_PM_OPS(amd_isp_dw_i2c_plat_suspend, amd_isp_dw_i2c_plat_resume) + RUNTIME_PM_OPS(amd_isp_dw_i2c_plat_runtime_suspend, amd_isp_dw_i2c_plat_runtime_resume, NULL) +}; + +/* Work with hotplug and coldplug */ +MODULE_ALIAS("platform:amd_isp_i2c_designware"); + +static struct platform_driver amd_isp_dw_i2c_driver = { + .probe = amd_isp_dw_i2c_plat_probe, + .remove = amd_isp_dw_i2c_plat_remove, + .driver = { + .name = DRV_NAME, + .pm = pm_ptr(&amd_isp_dw_i2c_dev_pm_ops), + }, +}; +module_platform_driver(amd_isp_dw_i2c_driver); + +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter in AMD ISP"); +MODULE_IMPORT_NS("I2C_DW"); +MODULE_IMPORT_NS("I2C_DW_COMMON"); +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>"); +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>"); +MODULE_AUTHOR("Bin Du <bin.du@amd.com>"); +MODULE_LICENSE("GPL");