| Message ID | 1449199466-6081-4-git-send-email-annie.wang@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Wang,
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.4-rc3 next-20151203]
url: https://github.com/0day-ci/linux/commits/Wang-Hongcheng/8250-AMD-Carrizo-UART-PL300-DMA-enablement/20151204-203235
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-lkp (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/acpi/acpi_apd.c: In function 'acpi_apd_create_device':
>> drivers/acpi/acpi_apd.c:145:3: error: implicit declaration of function 'setup_quirks' [-Werror=implicit-function-declaration]
setup_quirks(pdev, &amba_quirks);
^
cc1: some warnings being treated as errors
vim +/setup_quirks +145 drivers/acpi/acpi_apd.c
139 pdev = acpi_create_platform_device(adev);
140 if (IS_ERR_OR_NULL(pdev))
141 goto err_out;
142
143 if (!strncmp(pdev->name, "AMD0020", 7)) {
144 memset(&amba_quirks, 0, sizeof(amba_quirks));
> 145 setup_quirks(pdev, &amba_quirks);
146
147 amba_dev = acpi_create_amba_device(pdata->adev, 0x00041330,
148 48000000,
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, Dec 04, 2015 at 11:24:20AM +0800, Wang Hongcheng wrote: > AMD pl330 is a UART DMA device, it shares one ACPI item with UART. So > a platform device and an acpi device will be created according to > AMD0020 ACPI dev. And its mem base address must have an offset. As a > result, MULTI_ATTACHED_QUIRK and MULTI_ATTACHED_QUIRK are used. > > Signed-off-by: Wang Hongcheng <annie.wang@amd.com> > --- > drivers/acpi/acpi_amba.c | 31 +++++++++++++++++++++++---- > drivers/acpi/acpi_apd.c | 56 +++++++++++++++++++++++++++++++++++++----------- > include/linux/acpi.h | 13 +++++++++-- > 3 files changed, 81 insertions(+), 19 deletions(-) > > diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c > index 4f0366a..8a5269c 100644 > --- a/drivers/acpi/acpi_amba.c > +++ b/drivers/acpi/acpi_amba.c > @@ -31,6 +31,8 @@ ACPI_MODULE_NAME("amba"); > * @periphid: AMBA device periphid. > * @fixed_rate: Clock frequency. > * @pdata: Platform data specific to the device. > + * @quirk: Specific device config, including device multiattach. > + * and mem base offset. > * > * Check if the given @adev can be represented as an AMBA device and, if > * that's the case, create and register an AMBA device, populate its > @@ -42,7 +44,8 @@ ACPI_MODULE_NAME("amba"); > struct amba_device *acpi_create_amba_device(struct acpi_device *adev, > unsigned int periphid, > unsigned long fixed_rate, > - void *pdata) > + void *pdata, > + struct acpi_amba_quirk *quirk) > { > struct amba_device *amba_dev = NULL; > struct device *parent; > @@ -54,12 +57,14 @@ struct amba_device *acpi_create_amba_device(struct acpi_device *adev, > unsigned int i; > unsigned int irq[AMBA_NR_IRQS]; > struct clk *clk = ERR_PTR(-ENODEV); > + char amba_devname[100]; > > /* > * If the ACPI node already has a physical device attached, > - * skip it. > + * skip it. Except some special devices such as AMD0020 which > + * needs attach physical devices two times. > */ > - if (adev->physical_node_count) > + if (adev->physical_node_count && !(quirk->quirk & MULTI_ATTACHED_QUIRK)) > return NULL; > > INIT_LIST_HEAD(&resource_list); > @@ -85,7 +90,24 @@ struct amba_device *acpi_create_amba_device(struct acpi_device *adev, > memcpy(resource, rentry->res, sizeof(struct resource)); > } > > - amba_dev = amba_device_alloc(dev_name(&adev->dev), > + /* > + * The memory address of AMD pl330 has an offset of ACPI > + * mem resource. > + */ > + if (quirk->quirk & BASE_OFFSET_QUIRK) > + resource->start += quirk->base_offset; > + > + /* > + * If the ACPI device already has a node attached. It must be > + * renamed. > + */ > + if (quirk->quirk & MULTI_ATTACHED_QUIRK) > + sprintf(amba_devname, "%s%s", dev_name(&adev->dev), "DMA"); > + else > + memcpy(amba_devname, dev_name(&adev->dev), > + strlen(dev_name(&adev->dev))); > + > + amba_dev = amba_device_alloc(amba_devname, > resource->start, > resource_size(resource)); > Isn't this basially an MFD in a rather odd fashion? I would have though having a device which just splits the resources then creates 2 children would be a whole lot simpler? Graeme -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>-----Original Message----- >From: Graeme Gregory [mailto:gg@slimlogic.co.uk] >Sent: Friday, December 04, 2015 9:16 PM >To: Wang, Annie >Cc: Vinod Koul; Mika Westerberg; Joerg Roedel; Greg Kroah-Hartman; Rafael J. >Wysocki; linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux- >serial@vger.kernel.org; dmaengine@vger.kernel.org; iommu@lists.linux- >foundation.org; Borislav Petkov; Huang, Ray; Wan, Vincent; Xue, Ken; Li, Tony >Subject: Re: [PATCH 3/9] ACPI: add struct acpi_amba_quirk for AMD pl330 >specific device config > >On Fri, Dec 04, 2015 at 11:24:20AM +0800, Wang Hongcheng wrote: >> AMD pl330 is a UART DMA device, it shares one ACPI item with UART. So >> a platform device and an acpi device will be created according to >> AMD0020 ACPI dev. And its mem base address must have an offset. As a >> result, MULTI_ATTACHED_QUIRK and MULTI_ATTACHED_QUIRK are used. >> >> Signed-off-by: Wang Hongcheng <annie.wang@amd.com> >> --- >> drivers/acpi/acpi_amba.c | 31 +++++++++++++++++++++++---- >> drivers/acpi/acpi_apd.c | 56 +++++++++++++++++++++++++++++++++++++------ >----- >> include/linux/acpi.h | 13 +++++++++-- >> 3 files changed, 81 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c index >> 4f0366a..8a5269c 100644 >> --- a/drivers/acpi/acpi_amba.c >> +++ b/drivers/acpi/acpi_amba.c >> @@ -31,6 +31,8 @@ ACPI_MODULE_NAME("amba"); >> * @periphid: AMBA device periphid. >> * @fixed_rate: Clock frequency. >> * @pdata: Platform data specific to the device. >> + * @quirk: Specific device config, including device multiattach. >> + * and mem base offset. >> * >> * Check if the given @adev can be represented as an AMBA device and, if >> * that's the case, create and register an AMBA device, populate its >> @@ -42,7 +44,8 @@ ACPI_MODULE_NAME("amba"); struct amba_device >> *acpi_create_amba_device(struct acpi_device *adev, >> unsigned int periphid, >> unsigned long fixed_rate, >> - void *pdata) >> + void *pdata, >> + struct acpi_amba_quirk *quirk) >> { >> struct amba_device *amba_dev = NULL; >> struct device *parent; >> @@ -54,12 +57,14 @@ struct amba_device *acpi_create_amba_device(struct >acpi_device *adev, >> unsigned int i; >> unsigned int irq[AMBA_NR_IRQS]; >> struct clk *clk = ERR_PTR(-ENODEV); >> + char amba_devname[100]; >> >> /* >> * If the ACPI node already has a physical device attached, >> - * skip it. >> + * skip it. Except some special devices such as AMD0020 which >> + * needs attach physical devices two times. >> */ >> - if (adev->physical_node_count) >> + if (adev->physical_node_count && !(quirk->quirk & >> +MULTI_ATTACHED_QUIRK)) >> return NULL; >> >> INIT_LIST_HEAD(&resource_list); >> @@ -85,7 +90,24 @@ struct amba_device *acpi_create_amba_device(struct >acpi_device *adev, >> memcpy(resource, rentry->res, sizeof(struct resource)); >> } >> >> - amba_dev = amba_device_alloc(dev_name(&adev->dev), >> + /* >> + * The memory address of AMD pl330 has an offset of ACPI >> + * mem resource. >> + */ >> + if (quirk->quirk & BASE_OFFSET_QUIRK) >> + resource->start += quirk->base_offset; >> + >> + /* >> + * If the ACPI device already has a node attached. It must be >> + * renamed. >> + */ >> + if (quirk->quirk & MULTI_ATTACHED_QUIRK) >> + sprintf(amba_devname, "%s%s", dev_name(&adev->dev), >"DMA"); >> + else >> + memcpy(amba_devname, dev_name(&adev->dev), >> + strlen(dev_name(&adev->dev))); >> + >> + amba_dev = amba_device_alloc(amba_devname, >> resource->start, >> resource_size(resource)); >> > >Isn't this basially an MFD in a rather odd fashion? > >I would have though having a device which just splits the resources then creates 2 >children would be a whole lot simpler? > >Graeme It seems more complex, if I trans an ACPI device to pdev, then attach 2 platform child nodes, and create an amba device refer to one of the childs. Too many trans. Thanks, Hongcheng(Annie) -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 11, 2015 at 06:57:51AM +0000, Wang, Annie wrote: > >> + /* > >> + * If the ACPI device already has a node attached. It must be > >> + * renamed. > >> + */ > >> + if (quirk->quirk & MULTI_ATTACHED_QUIRK) > >> + sprintf(amba_devname, "%s%s", dev_name(&adev->dev), > >"DMA"); > >> + else > >> + memcpy(amba_devname, dev_name(&adev->dev), > >> + strlen(dev_name(&adev->dev))); > >> + > >> + amba_dev = amba_device_alloc(amba_devname, > >> resource->start, > >> resource_size(resource)); > >> > > > >Isn't this basially an MFD in a rather odd fashion? MFD yes, odd perhaps made out here! > > > >I would have though having a device which just splits the resources then creates 2 > >children would be a whole lot simpler? > > Yup! > > It seems more complex, if I trans an ACPI device to pdev, then attach 2 platform child nodes, > and create an amba device refer to one of the childs. Too many trans. Sorry but I dont think that is right assumption, it will simper and PM would become easy
diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c index 4f0366a..8a5269c 100644 --- a/drivers/acpi/acpi_amba.c +++ b/drivers/acpi/acpi_amba.c @@ -31,6 +31,8 @@ ACPI_MODULE_NAME("amba"); * @periphid: AMBA device periphid. * @fixed_rate: Clock frequency. * @pdata: Platform data specific to the device. + * @quirk: Specific device config, including device multiattach. + * and mem base offset. * * Check if the given @adev can be represented as an AMBA device and, if * that's the case, create and register an AMBA device, populate its @@ -42,7 +44,8 @@ ACPI_MODULE_NAME("amba"); struct amba_device *acpi_create_amba_device(struct acpi_device *adev, unsigned int periphid, unsigned long fixed_rate, - void *pdata) + void *pdata, + struct acpi_amba_quirk *quirk) { struct amba_device *amba_dev = NULL; struct device *parent; @@ -54,12 +57,14 @@ struct amba_device *acpi_create_amba_device(struct acpi_device *adev, unsigned int i; unsigned int irq[AMBA_NR_IRQS]; struct clk *clk = ERR_PTR(-ENODEV); + char amba_devname[100]; /* * If the ACPI node already has a physical device attached, - * skip it. + * skip it. Except some special devices such as AMD0020 which + * needs attach physical devices two times. */ - if (adev->physical_node_count) + if (adev->physical_node_count && !(quirk->quirk & MULTI_ATTACHED_QUIRK)) return NULL; INIT_LIST_HEAD(&resource_list); @@ -85,7 +90,24 @@ struct amba_device *acpi_create_amba_device(struct acpi_device *adev, memcpy(resource, rentry->res, sizeof(struct resource)); } - amba_dev = amba_device_alloc(dev_name(&adev->dev), + /* + * The memory address of AMD pl330 has an offset of ACPI + * mem resource. + */ + if (quirk->quirk & BASE_OFFSET_QUIRK) + resource->start += quirk->base_offset; + + /* + * If the ACPI device already has a node attached. It must be + * renamed. + */ + if (quirk->quirk & MULTI_ATTACHED_QUIRK) + sprintf(amba_devname, "%s%s", dev_name(&adev->dev), "DMA"); + else + memcpy(amba_devname, dev_name(&adev->dev), + strlen(dev_name(&adev->dev))); + + amba_dev = amba_device_alloc(amba_devname, resource->start, resource_size(resource)); @@ -136,6 +158,7 @@ struct amba_device *acpi_create_amba_device(struct acpi_device *adev, if (ret) goto amba_register_err; + amba_dev->dev.init_name = NULL; ret = amba_device_add(amba_dev, resource); if (ret) goto amba_register_err; diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c index a450e7a..eb3316a 100644 --- a/drivers/acpi/acpi_apd.c +++ b/drivers/acpi/acpi_apd.c @@ -3,7 +3,8 @@ * * Copyright (c) 2014,2015 AMD Corporation. * Authors: Ken Xue <Ken.Xue@amd.com> - * Wu, Jeff <Jeff.Wu@amd.com> + * Jeff Wu <15618388108@163.com> + * Wang Hongcheng <Annie.Wang@amd.com> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -17,6 +18,10 @@ #include <linux/acpi.h> #include <linux/err.h> #include <linux/pm.h> +#include <linux/amba/bus.h> +#include <linux/kernel.h> +#include <linux/sizes.h> +#include <linux/interrupt.h> #include "internal.h" @@ -31,14 +36,15 @@ struct apd_private_data; #define ACPI_APD_PM BIT(1) /** - * struct apd_device_desc - a descriptor for apd device - * @flags: device flags like %ACPI_APD_SYSFS, %ACPI_APD_PM + * struct apd_device_desc - a descriptor for apd device. + * @flags: device flags like %ACPI_APD_SYSFS, %ACPI_APD_PM; * @fixed_clk_rate: fixed rate input clock source for acpi device; - * 0 means no fixed rate input clock source - * @setup: a hook routine to set device resource during create platform device + *0 means no fixed rate input clock source; + * @clk_con_id: name of input clock source; + * @setup: a hook routine to set device resource during create platform device. * - * Device description defined as acpi_device_id.driver_data - */ + * Device description defined as acpi_device_id.driver_data. +*/ struct apd_device_desc { unsigned int flags; unsigned int fixed_clk_rate; @@ -71,6 +77,15 @@ static int acpi_apd_setup(struct apd_private_data *pdata) return 0; } +static void setup_quirks(struct platform_device *pdev, + struct acpi_amba_quirk *quirk) +{ + if (!strncmp(pdev->name, "AMD0020", 7)) { + quirk->quirk |= MULTI_ATTACHED_QUIRK | BASE_OFFSET_QUIRK; + quirk->base_offset = SZ_4K; + } +} + static struct apd_device_desc cz_i2c_desc = { .setup = acpi_apd_setup, .fixed_clk_rate = 133000000, @@ -88,15 +103,17 @@ static struct apd_device_desc cz_uart_desc = { #endif /* CONFIG_X86_AMD_PLATFORM_DEVICE */ /** -* Create platform device during acpi scan attach handle. -* Return value > 0 on success of creating device. -*/ + * Create platform device during acpi scan attach handle. + * Return value > 0 on success of creating device. + */ static int acpi_apd_create_device(struct acpi_device *adev, - const struct acpi_device_id *id) + const struct acpi_device_id *id) { const struct apd_device_desc *dev_desc = (void *)id->driver_data; struct apd_private_data *pdata; struct platform_device *pdev; + struct amba_device *amba_dev; + struct acpi_amba_quirk amba_quirks; int ret; if (!dev_desc) { @@ -118,9 +135,22 @@ static int acpi_apd_create_device(struct acpi_device *adev, } adev->driver_data = pdata; + pdev = acpi_create_platform_device(adev); - if (!IS_ERR_OR_NULL(pdev)) - return 1; + if (IS_ERR_OR_NULL(pdev)) + goto err_out; + + if (!strncmp(pdev->name, "AMD0020", 7)) { + memset(&amba_quirks, 0, sizeof(amba_quirks)); + setup_quirks(pdev, &amba_quirks); + + amba_dev = acpi_create_amba_device(pdata->adev, 0x00041330, + 48000000, + NULL, + &amba_quirks); + if (IS_ERR_OR_NULL(amba_dev)) + goto err_out; + } ret = PTR_ERR(pdev); adev->driver_data = NULL; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 04827d8..50961a5 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -468,19 +468,28 @@ void acpi_walk_dep_device_list(acpi_handle handle); struct platform_device *acpi_create_platform_device(struct acpi_device *); +struct acpi_amba_quirk { + u32 quirk; +#define MULTI_ATTACHED_QUIRK BIT(0) +#define BASE_OFFSET_QUIRK BIT(1) + u32 base_offset; +}; + #ifdef CONFIG_ARM_AMBA struct amba_device *acpi_create_amba_device(struct acpi_device *adev, unsigned int periphid, unsigned long fixed_rate, - void *pdata); + void *pdata, + struct acpi_amba_quirk *quirk); #else /* !CONFIG_ARM_AMBA */ static inline struct amba_device *acpi_create_amba_device(struct acpi_device *adev, unsigned int periphid, unsigned long fixed_rate, - void *pdata) + void *pdata, + struct acpi_amba_quirk *quirk) { return NULL; }
AMD pl330 is a UART DMA device, it shares one ACPI item with UART. So a platform device and an acpi device will be created according to AMD0020 ACPI dev. And its mem base address must have an offset. As a result, MULTI_ATTACHED_QUIRK and MULTI_ATTACHED_QUIRK are used. Signed-off-by: Wang Hongcheng <annie.wang@amd.com> --- drivers/acpi/acpi_amba.c | 31 +++++++++++++++++++++++---- drivers/acpi/acpi_apd.c | 56 +++++++++++++++++++++++++++++++++++++----------- include/linux/acpi.h | 13 +++++++++-- 3 files changed, 81 insertions(+), 19 deletions(-)