| Message ID | 20250228164519.3453927-1-pratap.nirujogi@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: amd-isp: Add ISP i2c-designware driver | expand |
Hi Mario, Please accept my apologies for the delayed response to your review comments. Thanks, Pratap On 2/28/2025 10:33 PM, Mario Limonciello wrote: > On 2/28/2025 10:45, 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. >> >> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> >> --- >> drivers/i2c/busses/Kconfig | 10 + >> drivers/i2c/busses/Makefile | 1 + >> drivers/i2c/busses/i2c-designware-amdisp.c | 266 +++++++++++++++++++++ >> drivers/i2c/busses/i2c-designware-amdisp.h | 24 ++ >> 4 files changed, 301 insertions(+) >> create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.c >> create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.h >> >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >> index fc438f445771..79448211baae 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 1c2a4510abe4..cfe53038df69 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..dc90510a440b >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-designware-amdisp.c >> @@ -0,0 +1,266 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright 2024-2025 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without >> limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be >> included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >> SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, >> DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/clk.h> >> +#include <linux/delay.h> >> +#include <linux/dmi.h> >> +#include <linux/err.h> >> +#include <linux/errno.h> >> +#include <linux/i2c.h> >> +#include <linux/interrupt.h> >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/property.h> >> +#include <linux/regmap.h> >> +#include <linux/reset.h> >> +#include <linux/sched.h> >> +#include <linux/slab.h> >> +#include <linux/suspend.h> >> +#include <linux/units.h> > > Can you please audit this list for accuracy? It seems like a very large > number of headers. > > Just off the bat for example I didn't see any DMI use, so dmi.h probably > isn't needed. I suspect there are others. > Thanks. Will clean-up headers files in V2. >> + >> +#include "i2c-designware-core.h" >> +#include "i2c-designware-amdisp.h" >> + >> +#define AMD_ISP_I2C_INPUT_CLK 100 //100 Mhz >> + >> +#define to_amd_isp_i2c_dev(dev) \ >> + ((struct amd_isp_i2c_dev *)container_of(dev, struct >> amd_isp_i2c_dev, dw_dev)) >> + >> +struct amd_isp_i2c_dev { >> + struct dw_i2c_dev dw_dev; >> +}; >> + >> +static void amd_isp_dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev) >> +{ >> + pm_runtime_disable(dev->dev); >> + >> + if (dev->shared_with_punit) >> + pm_runtime_put_noidle(dev->dev); >> +} >> + >> +static u32 amd_isp_dw_i2c_get_clk_rate(struct dw_i2c_dev *dev) >> +{ >> + return AMD_ISP_I2C_INPUT_CLK * 1000; >> +} >> + >> +static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev) >> +{ >> + struct i2c_adapter *adap; >> + struct amd_isp_i2c_dev *isp_i2c_dev; >> + struct dw_i2c_dev *dev; >> + int ret; >> + >> + isp_i2c_dev = devm_kzalloc(&pdev->dev, sizeof(struct >> amd_isp_i2c_dev), >> + GFP_KERNEL); >> + if (!isp_i2c_dev) >> + return -ENOMEM; >> + >> + dev = &isp_i2c_dev->dw_dev; >> + dev->dev = &pdev->dev; >> + >> + /** >> + * Use the polling mode to send/receive the data, because >> + * no IRQ connection from ISP I2C >> + */ >> + dev->flags |= ACCESS_POLLING; >> + platform_set_drvdata(pdev, dev); >> + >> + dev->base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(dev->base)) >> + return PTR_ERR(dev->base); >> + >> + ret = isp_power_set(true); >> + if (ret) { >> + dev_err(dev->dev, "unable to turn on the amdisp i2c power: >> %d\n", ret); >> + return ret; >> + } >> + >> + dev->get_clk_rate_khz = amd_isp_dw_i2c_get_clk_rate; >> + ret = i2c_dw_fw_parse_and_configure(dev); >> + if (ret) >> + goto exit; >> + >> + i2c_dw_configure(dev); >> + >> + adap = &dev->adapter; >> + adap->owner = THIS_MODULE; >> + ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); >> + adap->dev.of_node = pdev->dev.of_node; >> + /* arbitrary large number to avoid any conflicts */ >> + adap->nr = 99; > > Would it be better to allocate an IDA here? > Thanks. Will remove this hardcoding and use the dynamically assigned bus id in V2. >> + >> + if (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); >> + >> + /* The code below assumes runtime PM to be disabled. */ >> + WARN_ON(pm_runtime_enabled(&pdev->dev)); >> + >> + pm_runtime_dont_use_autosuspend(&pdev->dev); >> + pm_runtime_set_active(&pdev->dev); >> + >> + if (dev->shared_with_punit) >> + pm_runtime_get_noresume(&pdev->dev); >> + >> + pm_runtime_enable(&pdev->dev); >> + >> + ret = i2c_dw_probe(dev); >> + if (ret) { >> + dev_err(dev->dev, "i2c_dw_probe failed %d\n", ret); >> + goto exit_probe; >> + } >> + >> + isp_power_set(false); >> + return ret; >> + >> +exit_probe: >> + amd_isp_dw_i2c_plat_pm_cleanup(dev); >> + isp_power_set(false); >> +exit: >> + isp_power_set(false); > > I see 3 cases that isp_power_set(false) is called above: > * success > * failure exit probe > * failure exit > I should have removed isp_power_set() references before submitting the patch. Sorry its overlooked. isp_power_set() is the exported symbol from ISP driver. Will take care of removing the dependency on isp_power_set() in V2. > exit_probe falls through to exit, which means it's called twice. That > seems to be a mistake to me. > Thanks of identifying the bug with "exit_probe" and "exit", will fix it in V2. > But since it's also called in the success flow this makes me wonder if > it's better to use a macro like _free() which will automatically call > isp_power_set(false) when the function exits. > > Furthermore; are you missing a call to isp_power_set(true) in runtime > resume and isp_power_set(false) in runtime suspend? It would seem > logical to me that when runtime suspended the tile is off and when on > runtime resumed it's back on. > Instead of direct call to ISP driver function, will add the ISP device to generic power domain (gpd) to control power remotely using pm runtime calls. >> + return ret; >> +} >> + >> +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) >> +{ >> + struct dw_i2c_dev *dev = platform_get_drvdata(pdev); >> + >> + pm_runtime_get_sync(&pdev->dev); >> + >> + i2c_del_adapter(&dev->adapter); >> + >> + i2c_dw_disable(dev); >> + >> + pm_runtime_dont_use_autosuspend(&pdev->dev); >> + pm_runtime_put_sync(&pdev->dev); >> + amd_isp_dw_i2c_plat_pm_cleanup(dev); >> + >> + reset_control_assert(dev->rst); >> +} >> + >> +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); >> + >> + i2c_mark_adapter_suspended(&i_dev->adapter); >> + >> + return amd_isp_dw_i2c_plat_runtime_suspend(dev); >> +} >> + >> +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->shared_with_punit) >> + i2c_dw_prepare_clk(i_dev, true); >> + >> + 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 = "amd_isp_i2c_designware", >> + .pm = pm_ptr(&amd_isp_dw_i2c_dev_pm_ops), >> + }, >> +}; >> + >> +static int __init amd_isp_dw_i2c_init_driver(void) >> +{ >> + return platform_driver_register(&amd_isp_dw_i2c_driver); >> +} >> +subsys_initcall(amd_isp_dw_i2c_init_driver); >> + >> +static void __exit amd_isp_dw_i2c_exit_driver(void) >> +{ >> + platform_driver_unregister(&amd_isp_dw_i2c_driver); >> +} >> +module_exit(amd_isp_dw_i2c_exit_driver); >> + >> +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>"); >> +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>"); >> +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter in AMD ISP"); >> +MODULE_LICENSE("GPL"); >> +MODULE_IMPORT_NS("I2C_DW"); >> +MODULE_IMPORT_NS("I2C_DW_COMMON"); >> +MODULE_LICENSE("GPL and additional rights"); >> diff --git a/drivers/i2c/busses/i2c-designware-amdisp.h b/drivers/i2c/ >> busses/i2c-designware-amdisp.h >> new file mode 100644 >> index 000000000000..f98661fdaedf >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-designware-amdisp.h >> @@ -0,0 +1,24 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright 2024-2025 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without >> limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be >> included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >> SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, >> DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. >> + */ >> + >> +int isp_power_set(int on); > > Where does this symbol actually come from? I didn't see it in this series. Sorry for the confusion caused. This symbol is coming from ISP driver which is not part of this series. We are planning to submit ISP driver patches after the completion of platform and sensor driver patches review.
Hi CJ, Please accept my apologies for the delayed response to your review comments. Thanks, Pratap On 3/1/2025 3:32 AM, Christophe JAILLET wrote: > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > Le 28/02/2025 à 17:45, 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. >> >> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> > > ... > >> diff --git a/drivers/i2c/busses/i2c-designware-amdisp.c b/drivers/i2c/ >> busses/i2c-designware-amdisp.c >> new file mode 100644 >> index 000000000000..dc90510a440b >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-designware-amdisp.c >> @@ -0,0 +1,266 @@ >> +/* SPDX-License-Identifier: MIT */ > > I think that this should be // comment style for SPDX-License-Identifier > en c files. > Thanks. Will update in V2. >> +/* >> + * Copyright 2024-2025 Advanced Micro Devices, Inc. >> + * > > ... > >> +static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev) >> +{ >> + struct i2c_adapter *adap; >> + struct amd_isp_i2c_dev *isp_i2c_dev; >> + struct dw_i2c_dev *dev; >> + int ret; >> + >> + isp_i2c_dev = devm_kzalloc(&pdev->dev, sizeof(struct >> amd_isp_i2c_dev), > > sizeof(*isp_i2c_dev) maybe? > Thanks. Will update in V2. >> + GFP_KERNEL); >> + if (!isp_i2c_dev) >> + return -ENOMEM; >> + >> + dev = &isp_i2c_dev->dw_dev; >> + dev->dev = &pdev->dev; >> + >> + /** > > Just /* > Thanks. Will fix this in V2. >> + * Use the polling mode to send/receive the data, because >> + * no IRQ connection from ISP I2C >> + */ >> + dev->flags |= ACCESS_POLLING; >> + platform_set_drvdata(pdev, dev); >> + >> + dev->base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(dev->base)) >> + return PTR_ERR(dev->base); >> + >> + ret = isp_power_set(true); >> + if (ret) { >> + dev_err(dev->dev, "unable to turn on the amdisp i2c >> power:%d\n", ret); > > return dev_err_probe() would make code slightly simpler. > Thanks. Will update in V2. >> + return ret; >> + } >> + >> + dev->get_clk_rate_khz = amd_isp_dw_i2c_get_clk_rate; >> + ret = i2c_dw_fw_parse_and_configure(dev); >> + if (ret) >> + goto exit; >> + >> + i2c_dw_configure(dev); >> + >> + adap = &dev->adapter; >> + adap->owner = THIS_MODULE; >> + ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); >> + adap->dev.of_node = pdev->dev.of_node; >> + /* arbitrary large number to avoid any conflicts */ >> + adap->nr = 99; >> + >> + if (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); >> + } > > Unneeded { } in both branches. > Thanks. Will remove them in V2. >> + >> + device_enable_async_suspend(&pdev->dev); >> + >> + /* The code below assumes runtime PM to be disabled. */ >> + WARN_ON(pm_runtime_enabled(&pdev->dev)); >> + >> + pm_runtime_dont_use_autosuspend(&pdev->dev); >> + pm_runtime_set_active(&pdev->dev); >> + >> + if (dev->shared_with_punit) >> + pm_runtime_get_noresume(&pdev->dev); >> + >> + pm_runtime_enable(&pdev->dev); >> + >> + ret = i2c_dw_probe(dev); >> + if (ret) { >> + dev_err(dev->dev, "i2c_dw_probe failed %d\n", ret); > > dev_err_probe() would make code slightly simpler. > Thanks. Will update in V2. >> + goto exit_probe; >> + } >> + >> + isp_power_set(false); >> + return ret; >> + >> +exit_probe: >> + amd_isp_dw_i2c_plat_pm_cleanup(dev); >> + isp_power_set(false); >> +exit: >> + isp_power_set(false); >> + return ret; >> +} >> + >> +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) >> +{ >> + struct dw_i2c_dev *dev = platform_get_drvdata(pdev); >> + >> + pm_runtime_get_sync(&pdev->dev); >> + >> + i2c_del_adapter(&dev->adapter); >> + >> + i2c_dw_disable(dev); >> + >> + pm_runtime_dont_use_autosuspend(&pdev->dev); >> + pm_runtime_put_sync(&pdev->dev); >> + amd_isp_dw_i2c_plat_pm_cleanup(dev); >> + >> + reset_control_assert(dev->rst); > > Is it needed? (there is apparently no reset_control_deassert() in this > driver) > Thanks. Its not needed, will remove it in V2. >> +} > > ... > >> +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>"); >> +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>"); >> +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter in AMD ISP"); >> +MODULE_LICENSE("GPL"); > > MIT is stated in SPDX-License-Identifier > Thanks. Will update the license info. >> +MODULE_IMPORT_NS("I2C_DW"); >> +MODULE_IMPORT_NS("I2C_DW_COMMON"); >> +MODULE_LICENSE("GPL and additional rights"); > > Is it allowed to have several MODULE_LICENSE? > Thanks. Will use GPL alone in V2. > ... > > CJ
Hi Krzysztof, Please accept my apologies for the delayed response to your review comments. Thanks, Pratap On 3/1/2025 8:26 AM, Krzysztof Kozlowski wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On 28/02/2025 17:45, Pratap Nirujogi wrote: >> 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 1c2a4510abe4..cfe53038df69 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..dc90510a440b >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-designware-amdisp.c >> @@ -0,0 +1,266 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright 2024-2025 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> + * copy of this software and associated documentation files (the "Software"), >> + * to deal in the Software without restriction, including without limitation >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. > > Don't add custom license boilerplate. > Thanks. Will remove it in V2. > >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/clk.h> >> +#include <linux/delay.h> >> +#include <linux/dmi.h> >> +#include <linux/err.h> >> +#include <linux/errno.h> >> +#include <linux/i2c.h> >> +#include <linux/interrupt.h> >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/property.h> >> +#include <linux/regmap.h> >> +#include <linux/reset.h> >> +#include <linux/sched.h> >> +#include <linux/slab.h> >> +#include <linux/suspend.h> >> +#include <linux/units.h> >> + >> +#include "i2c-designware-core.h" >> +#include "i2c-designware-amdisp.h" >> + >> +#define AMD_ISP_I2C_INPUT_CLK 100 //100 Mhz >> + >> +#define to_amd_isp_i2c_dev(dev) \ >> + ((struct amd_isp_i2c_dev *)container_of(dev, struct amd_isp_i2c_dev, dw_dev)) > > Why do you need to cast? To drop const? That's poor coding. > Thanks. Will remove it in V2. >> + >> +struct amd_isp_i2c_dev { >> + struct dw_i2c_dev dw_dev; >> +}; >> + >> +static void amd_isp_dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev) >> +{ >> + pm_runtime_disable(dev->dev); >> + >> + if (dev->shared_with_punit) >> + pm_runtime_put_noidle(dev->dev); >> +} >> + >> +static u32 amd_isp_dw_i2c_get_clk_rate(struct dw_i2c_dev *dev) >> +{ >> + return AMD_ISP_I2C_INPUT_CLK * 1000; >> +} >> + >> +static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev) >> +{ >> + struct i2c_adapter *adap; >> + struct amd_isp_i2c_dev *isp_i2c_dev; >> + struct dw_i2c_dev *dev; >> + int ret; >> + >> + isp_i2c_dev = devm_kzalloc(&pdev->dev, sizeof(struct amd_isp_i2c_dev), > > sizeof(*) > >> + GFP_KERNEL); >> + if (!isp_i2c_dev) >> + return -ENOMEM; >> + >> + dev = &isp_i2c_dev->dw_dev; >> + dev->dev = &pdev->dev; >> + >> + /** > > Not a kerneldoc. > > Please run standard kernel tools for static analysis, like coccinelle, > smatch and sparse, and fix reported warnings. Also please check for > warnings when building with W=1. Most of these commands (checks or W=1 > build) can build specific targets, like some directory, to narrow the > scope to only your code. The code here looks like it needs a fix. Feel > free to get in touch if the warning is not clear. > Thanks. Will run the suggested tools and fix the warnings reported in V2. >> + * Use the polling mode to send/receive the data, because >> + * no IRQ connection from ISP I2C >> + */ >> + dev->flags |= ACCESS_POLLING; >> + platform_set_drvdata(pdev, dev); >> + >> + dev->base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(dev->base)) >> + return PTR_ERR(dev->base); >> + >> + ret = isp_power_set(true); >> + if (ret) { >> + dev_err(dev->dev, "unable to turn on the amdisp i2c power:%d\n", ret); > > Syntax is return dev_err_probe() > Thanks. Will update in V2. >> + return ret; >> + } >> + >> + dev->get_clk_rate_khz = amd_isp_dw_i2c_get_clk_rate; >> + ret = i2c_dw_fw_parse_and_configure(dev); >> + if (ret) >> + goto exit; >> + >> + i2c_dw_configure(dev); >> + >> + adap = &dev->adapter; >> + adap->owner = THIS_MODULE; >> + ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); >> + adap->dev.of_node = pdev->dev.of_node; >> + /* arbitrary large number to avoid any conflicts */ >> + adap->nr = 99; >> + >> + if (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); >> + >> + /* The code below assumes runtime PM to be disabled. */ >> + WARN_ON(pm_runtime_enabled(&pdev->dev)); > > And how it could be enabled? Drop or fix your driver. > Thanks. Will drop this in V2. >> + >> + pm_runtime_dont_use_autosuspend(&pdev->dev); >> + pm_runtime_set_active(&pdev->dev); >> + >> + if (dev->shared_with_punit) >> + pm_runtime_get_noresume(&pdev->dev); >> + >> + pm_runtime_enable(&pdev->dev); >> + >> + ret = i2c_dw_probe(dev); >> + if (ret) { >> + dev_err(dev->dev, "i2c_dw_probe failed %d\n", ret); > > Use dev_err_probe() > Thanks. Will update in V2. >> + goto exit_probe; >> + } >> + >> + isp_power_set(false); >> + return ret; >> + >> +exit_probe: >> + amd_isp_dw_i2c_plat_pm_cleanup(dev); >> + isp_power_set(false); >> +exit: >> + isp_power_set(false); >> + return ret; >> +} >> + >> +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) >> +{ >> + struct dw_i2c_dev *dev = platform_get_drvdata(pdev); >> + >> + pm_runtime_get_sync(&pdev->dev); >> + >> + i2c_del_adapter(&dev->adapter); >> + >> + i2c_dw_disable(dev); >> + >> + pm_runtime_dont_use_autosuspend(&pdev->dev); >> + pm_runtime_put_sync(&pdev->dev); >> + amd_isp_dw_i2c_plat_pm_cleanup(dev); >> + >> + reset_control_assert(dev->rst); >> +} >> + >> +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); >> + >> + i2c_mark_adapter_suspended(&i_dev->adapter); >> + >> + return amd_isp_dw_i2c_plat_runtime_suspend(dev); >> +} >> + >> +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->shared_with_punit) >> + i2c_dw_prepare_clk(i_dev, true); >> + >> + 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 = "amd_isp_i2c_designware", >> + .pm = pm_ptr(&amd_isp_dw_i2c_dev_pm_ops), >> + }, >> +}; >> + >> +static int __init amd_isp_dw_i2c_init_driver(void) >> +{ >> + return platform_driver_register(&amd_isp_dw_i2c_driver); >> +} >> +subsys_initcall(amd_isp_dw_i2c_init_driver); > > Why this cannot be standard module initcall? This is ISP, not a critical > boot component. > Thanks. It need not be subsys_initcall. Will use module_init in V2. >> + >> +static void __exit amd_isp_dw_i2c_exit_driver(void) >> +{ >> + platform_driver_unregister(&amd_isp_dw_i2c_driver); >> +} >> +module_exit(amd_isp_dw_i2c_exit_driver); >> + >> +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>"); >> +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>"); >> +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter in AMD ISP"); >> +MODULE_LICENSE("GPL"); >> +MODULE_IMPORT_NS("I2C_DW"); >> +MODULE_IMPORT_NS("I2C_DW_COMMON"); >> +MODULE_LICENSE("GPL and additional rights"); >> diff --git a/drivers/i2c/busses/i2c-designware-amdisp.h b/drivers/i2c/busses/i2c-designware-amdisp.h >> new file mode 100644 >> index 000000000000..f98661fdaedf >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-designware-amdisp.h >> @@ -0,0 +1,24 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright 2024-2025 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> + * copy of this software and associated documentation files (the "Software"), >> + * to deal in the Software without restriction, including without limitation >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. > > Don't add custom license boilerplate. > Thanks. Will remove it in V2. >> + */ >> + >> +int isp_power_set(int on); > > > Best regards, > Krzysztof
On 3/1/2025 8:33 AM, Krzysztof Kozlowski wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On 28/02/2025 17:45, 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. >> >> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> >> --- >> drivers/i2c/busses/Kconfig | 10 + >> drivers/i2c/busses/Makefile | 1 + >> drivers/i2c/busses/i2c-designware-amdisp.c | 266 +++++++++++++++++++++ >> drivers/i2c/busses/i2c-designware-amdisp.h | 24 ++ >> 4 files changed, 301 insertions(+) >> create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.c >> create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.h >> >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >> index fc438f445771..79448211baae 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 1c2a4510abe4..cfe53038df69 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..dc90510a440b >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-designware-amdisp.c >> @@ -0,0 +1,266 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright 2024-2025 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> + * copy of this software and associated documentation files (the "Software"), >> + * to deal in the Software without restriction, including without limitation >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/clk.h> >> +#include <linux/delay.h> >> +#include <linux/dmi.h> >> +#include <linux/err.h> >> +#include <linux/errno.h> > > Hm? > >> +#include <linux/i2c.h> >> +#include <linux/interrupt.h> >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/module.h> >> +#include <linux/of.h> > > Drop... or you miss bindings. > > Many more headers look not used or even wrong. > Thanks. Will clean-up header files in V2. > Best regards, > Krzysztof
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index fc438f445771..79448211baae 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 1c2a4510abe4..cfe53038df69 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..dc90510a440b --- /dev/null +++ b/drivers/i2c/busses/i2c-designware-amdisp.c @@ -0,0 +1,266 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright 2024-2025 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +#include <linux/clk-provider.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/dmi.h> +#include <linux/err.h> +#include <linux/errno.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> +#include <linux/property.h> +#include <linux/regmap.h> +#include <linux/reset.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/suspend.h> +#include <linux/units.h> + +#include "i2c-designware-core.h" +#include "i2c-designware-amdisp.h" + +#define AMD_ISP_I2C_INPUT_CLK 100 //100 Mhz + +#define to_amd_isp_i2c_dev(dev) \ + ((struct amd_isp_i2c_dev *)container_of(dev, struct amd_isp_i2c_dev, dw_dev)) + +struct amd_isp_i2c_dev { + struct dw_i2c_dev dw_dev; +}; + +static void amd_isp_dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev) +{ + pm_runtime_disable(dev->dev); + + if (dev->shared_with_punit) + pm_runtime_put_noidle(dev->dev); +} + +static u32 amd_isp_dw_i2c_get_clk_rate(struct dw_i2c_dev *dev) +{ + return AMD_ISP_I2C_INPUT_CLK * 1000; +} + +static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev) +{ + struct i2c_adapter *adap; + struct amd_isp_i2c_dev *isp_i2c_dev; + struct dw_i2c_dev *dev; + int ret; + + isp_i2c_dev = devm_kzalloc(&pdev->dev, sizeof(struct amd_isp_i2c_dev), + GFP_KERNEL); + if (!isp_i2c_dev) + return -ENOMEM; + + dev = &isp_i2c_dev->dw_dev; + dev->dev = &pdev->dev; + + /** + * Use the polling mode to send/receive the data, because + * no IRQ connection from ISP I2C + */ + dev->flags |= ACCESS_POLLING; + platform_set_drvdata(pdev, dev); + + dev->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(dev->base)) + return PTR_ERR(dev->base); + + ret = isp_power_set(true); + if (ret) { + dev_err(dev->dev, "unable to turn on the amdisp i2c power:%d\n", ret); + return ret; + } + + dev->get_clk_rate_khz = amd_isp_dw_i2c_get_clk_rate; + ret = i2c_dw_fw_parse_and_configure(dev); + if (ret) + goto exit; + + i2c_dw_configure(dev); + + adap = &dev->adapter; + adap->owner = THIS_MODULE; + ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); + adap->dev.of_node = pdev->dev.of_node; + /* arbitrary large number to avoid any conflicts */ + adap->nr = 99; + + if (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); + + /* The code below assumes runtime PM to be disabled. */ + WARN_ON(pm_runtime_enabled(&pdev->dev)); + + pm_runtime_dont_use_autosuspend(&pdev->dev); + pm_runtime_set_active(&pdev->dev); + + if (dev->shared_with_punit) + pm_runtime_get_noresume(&pdev->dev); + + pm_runtime_enable(&pdev->dev); + + ret = i2c_dw_probe(dev); + if (ret) { + dev_err(dev->dev, "i2c_dw_probe failed %d\n", ret); + goto exit_probe; + } + + isp_power_set(false); + return ret; + +exit_probe: + amd_isp_dw_i2c_plat_pm_cleanup(dev); + isp_power_set(false); +exit: + isp_power_set(false); + return ret; +} + +static void amd_isp_dw_i2c_plat_remove(struct platform_device *pdev) +{ + struct dw_i2c_dev *dev = platform_get_drvdata(pdev); + + pm_runtime_get_sync(&pdev->dev); + + i2c_del_adapter(&dev->adapter); + + i2c_dw_disable(dev); + + pm_runtime_dont_use_autosuspend(&pdev->dev); + pm_runtime_put_sync(&pdev->dev); + amd_isp_dw_i2c_plat_pm_cleanup(dev); + + reset_control_assert(dev->rst); +} + +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); + + i2c_mark_adapter_suspended(&i_dev->adapter); + + return amd_isp_dw_i2c_plat_runtime_suspend(dev); +} + +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->shared_with_punit) + i2c_dw_prepare_clk(i_dev, true); + + 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 = "amd_isp_i2c_designware", + .pm = pm_ptr(&amd_isp_dw_i2c_dev_pm_ops), + }, +}; + +static int __init amd_isp_dw_i2c_init_driver(void) +{ + return platform_driver_register(&amd_isp_dw_i2c_driver); +} +subsys_initcall(amd_isp_dw_i2c_init_driver); + +static void __exit amd_isp_dw_i2c_exit_driver(void) +{ + platform_driver_unregister(&amd_isp_dw_i2c_driver); +} +module_exit(amd_isp_dw_i2c_exit_driver); + +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>"); +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>"); +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter in AMD ISP"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS("I2C_DW"); +MODULE_IMPORT_NS("I2C_DW_COMMON"); +MODULE_LICENSE("GPL and additional rights"); diff --git a/drivers/i2c/busses/i2c-designware-amdisp.h b/drivers/i2c/busses/i2c-designware-amdisp.h new file mode 100644 index 000000000000..f98661fdaedf --- /dev/null +++ b/drivers/i2c/busses/i2c-designware-amdisp.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright 2024-2025 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +int isp_power_set(int on);
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. Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com> --- drivers/i2c/busses/Kconfig | 10 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-designware-amdisp.c | 266 +++++++++++++++++++++ drivers/i2c/busses/i2c-designware-amdisp.h | 24 ++ 4 files changed, 301 insertions(+) create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.c create mode 100644 drivers/i2c/busses/i2c-designware-amdisp.h