| Message ID | 1449771661-40300-1-git-send-email-qipeng.zha@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Fri, 2015-12-11 at 02:21 +0800, Qipeng Zha wrote: > BIOS restructure exported memory resources for Punit > in acpi table, So update resources for Punit. > > Signed-off-by: Qipeng Zha <qipeng.zha@intel.com> > Almost. See below. > --- > change in v9 > rename indexes of all acpi resources; > add comments for punit_res; > --- > drivers/platform/x86/intel_pmc_ipc.c | 114 ++++++++++++++++++++++++- > ---------- > 1 file changed, 80 insertions(+), 34 deletions(-) > > diff --git a/drivers/platform/x86/intel_pmc_ipc.c > b/drivers/platform/x86/intel_pmc_ipc.c > index 28b2a12..00812a4 100644 > --- a/drivers/platform/x86/intel_pmc_ipc.c > +++ b/drivers/platform/x86/intel_pmc_ipc.c > @@ -68,8 +68,12 @@ > #define PLAT_RESOURCE_IPC_INDEX 0 > #define PLAT_RESOURCE_IPC_SIZE 0x1000 > #define PLAT_RESOURCE_GCR_SIZE 0x1000 > -#define PLAT_RESOURCE_PUNIT_DATA_INDEX 1 > -#define PLAT_RESOURCE_PUNIT_INTER_INDEX 2 > +#define PLAT_RESOURCE_BIOS_DATA_INDEX 1 > +#define PLAT_RESOURCE_BIOS_INTER_INDEX 2 Since there is a comment and new version will be anticipated I also would like to propose to change INTER to IFACE because latter more particular. Darren? > +#define PLAT_RESOURCE_ISP_DATA_INDEX 4 > +#define PLAT_RESOURCE_ISP_INTER_INDEX 5 > +#define PLAT_RESOURCE_GTD_DATA_INDEX 6 > +#define PLAT_RESOURCE_GTD_INTER_INDEX 7 > #define PLAT_RESOURCE_ACPI_IO_INDEX 0 > > /* > @@ -105,10 +109,6 @@ static struct intel_pmc_ipc_dev { > int gcr_size; > > /* punit */ > - resource_size_t punit_base; > - int punit_size; > - resource_size_t punit_base2; > - int punit_size2; > struct platform_device *punit_dev; > } ipcdev; > > @@ -444,9 +444,22 @@ static const struct attribute_group > intel_ipc_group = { > .attrs = intel_ipc_attrs, > }; > > -#define PUNIT_RESOURCE_INTER 1 > -static struct resource punit_res[] = { > - /* Punit */ > +static struct resource punit_res_array[] = { > + /* Punit BIOS */ > + { > + .flags = IORESOURCE_MEM, > + }, > + { > + .flags = IORESOURCE_MEM, > + }, > + /* Punit ISP */ > + { > + .flags = IORESOURCE_MEM, > + }, > + { > + .flags = IORESOURCE_MEM, > + }, > + /* Punit GTD */ > { > .flags = IORESOURCE_MEM, > }, > @@ -481,7 +494,6 @@ static struct itco_wdt_platform_data tco_info = { > static int ipc_create_punit_device(void) > { > struct platform_device *pdev; > - struct resource *res; > int ret; > > pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1); > @@ -491,17 +503,8 @@ static int ipc_create_punit_device(void) > } > > pdev->dev.parent = ipcdev.dev; > - > - res = punit_res; > - res->start = ipcdev.punit_base; > - res->end = res->start + ipcdev.punit_size - 1; > - > - res = punit_res + PUNIT_RESOURCE_INTER; > - res->start = ipcdev.punit_base2; > - res->end = res->start + ipcdev.punit_size2 - 1; > - > - ret = platform_device_add_resources(pdev, punit_res, > - ARRAY_SIZE(punit_res)); > + ret = platform_device_add_resources(pdev, punit_res_array, > + ARRAY_SIZE(punit_res_arr > ay)); > if (ret) { > dev_err(ipcdev.dev, "Failed to add platform punit > resources\n"); > goto err; > @@ -590,7 +593,7 @@ static int ipc_create_pmc_devices(void) > > static int ipc_plat_get_res(struct platform_device *pdev) > { > - struct resource *res; > + struct resource *res, *punit_res; > void __iomem *addr; > int size; > > @@ -606,28 +609,71 @@ static int ipc_plat_get_res(struct > platform_device *pdev) > dev_info(&pdev->dev, "io res: %llx %x\n", > (long long)res->start, (int)resource_size(res)); > > + /* This is index 0 to cover BIOS data register */ > + punit_res = punit_res_array; > res = platform_get_resource(pdev, IORESOURCE_MEM, > - PLAT_RESOURCE_PUNIT_DATA_INDEX); > + PLAT_RESOURCE_BIOS_DATA_INDEX); > if (!res) { > - dev_err(&pdev->dev, "Failed to get punit > resource\n"); > + dev_err(&pdev->dev, "Failed to get res of punit BIOS > data\n"); > return -ENXIO; > } > - size = resource_size(res); > - ipcdev.punit_base = res->start; > - ipcdev.punit_size = size; > - dev_info(&pdev->dev, "punit data res: %llx %x\n", > + *punit_res = *res; > + dev_info(&pdev->dev, "punit BIOS data res: %llx %x\n", > (long long)res->start, (int)resource_size(res)); > > res = platform_get_resource(pdev, IORESOURCE_MEM, > - PLAT_RESOURCE_PUNIT_INTER_INDEX) > ; > + PLAT_RESOURCE_BIOS_INTER_INDEX); > if (!res) { > - dev_err(&pdev->dev, "Failed to get punit inter > resource\n"); > + dev_err(&pdev->dev, "Failed to get res of punit BIOS > inter\n"); > return -ENXIO; > } > - size = resource_size(res); > - ipcdev.punit_base2 = res->start; > - ipcdev.punit_size2 = size; > - dev_info(&pdev->dev, "punit interface res: %llx %x\n", > + /* This is index 1 to cover BIOS interface register */ > + *++punit_res = *res; > + dev_info(&pdev->dev, "punit BIOS interface res: %llx %x\n", > + (long long)res->start, (int)resource_size(res)); There is a specifier to print struct resource, please use it instead. %pR IIRC. Same for all similar cases below. > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, > + PLAT_RESOURCE_ISP_DATA_INDEX); > + if (!res) { > + dev_err(&pdev->dev, "Failed to get res of punit ISP > data\n"); > + return -ENXIO; > + } > + /* This is index 2 to cover ISP data register */ > + *++punit_res = *res; > + dev_info(&pdev->dev, "punit ISP data res: %llx %x\n", > + (long long)res->start, (int)resource_size(res)); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, > + PLAT_RESOURCE_ISP_INTER_INDEX); > + if (!res) { > + dev_err(&pdev->dev, "Failed to get res of punit ISP > inter\n"); > + return -ENXIO; > + } > + /* This is index 3 to cover ISP interface register */ > + *++punit_res = *res; > + dev_info(&pdev->dev, "punit ISP interface res: %llx %x\n", > + (long long)res->start, (int)resource_size(res)); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, > + PLAT_RESOURCE_GTD_DATA_INDEX); > + if (!res) { > + dev_err(&pdev->dev, "Failed to get res of punit GTD > data\n"); > + return -ENXIO; > + } > + /* This is index 4 to cover GTD data register */ > + *++punit_res = *res; > + dev_info(&pdev->dev, "punit GTD data res: %llx %x\n", > + (long long)res->start, (int)resource_size(res)); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, > + PLAT_RESOURCE_GTD_INTER_INDEX); > + if (!res) { > + dev_err(&pdev->dev, "Failed to get res of punit GTD > inter\n"); > + return -ENXIO; > + } > + /* This is index 5 to cover GTD interface register */ > + *++punit_res = *res; > + dev_info(&pdev->dev, "punit GTD interface res: %llx %x\n", > (long long)res->start, (int)resource_size(res)); > > res = platform_get_resource(pdev, IORESOURCE_MEM,
On Thu, Dec 10, 2015 at 01:13:01PM +0200, Andy Shevchenko wrote: > On Fri, 2015-12-11 at 02:21 +0800, Qipeng Zha wrote: > > BIOS restructure exported memory resources for Punit > > in acpi table, So update resources for Punit. > > > > Signed-off-by: Qipeng Zha <qipeng.zha@intel.com> > > > > Almost. See below. > > > --- > > change in v9 > > rename indexes of all acpi resources; > > add comments for punit_res; > > --- > > drivers/platform/x86/intel_pmc_ipc.c | 114 ++++++++++++++++++++++++- > > ---------- > > 1 file changed, 80 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/platform/x86/intel_pmc_ipc.c > > b/drivers/platform/x86/intel_pmc_ipc.c > > index 28b2a12..00812a4 100644 > > --- a/drivers/platform/x86/intel_pmc_ipc.c > > +++ b/drivers/platform/x86/intel_pmc_ipc.c > > @@ -68,8 +68,12 @@ > > #define PLAT_RESOURCE_IPC_INDEX 0 > > #define PLAT_RESOURCE_IPC_SIZE 0x1000 > > #define PLAT_RESOURCE_GCR_SIZE 0x1000 > > -#define PLAT_RESOURCE_PUNIT_DATA_INDEX 1 > > -#define PLAT_RESOURCE_PUNIT_INTER_INDEX 2 > > +#define PLAT_RESOURCE_BIOS_DATA_INDEX 1 > > +#define PLAT_RESOURCE_BIOS_INTER_INDEX 2 > > Since there is a comment and new version will be anticipated I also > would like to propose to change INTER to IFACE because latter more > particular. Darren? Yes, I had the same thought, but wasn't going to request the change just for this. Andriy did point out the pointer format below previously however. Please do correct that - updating INTER to IFACE would be a welcome improvement as well. > > - size = resource_size(res); > > - ipcdev.punit_base2 = res->start; > > - ipcdev.punit_size2 = size; > > - dev_info(&pdev->dev, "punit interface res: %llx %x\n", > > + /* This is index 1 to cover BIOS interface register */ > > + *++punit_res = *res; > > + dev_info(&pdev->dev, "punit BIOS interface res: %llx %x\n", > > + (long long)res->start, (int)resource_size(res)); > > There is a specifier to print struct resource, please use it instead. > %pR IIRC. > Yes, %pR, see Documentation/printk-formats.txt > Same for all similar cases below. >
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c index 28b2a12..00812a4 100644 --- a/drivers/platform/x86/intel_pmc_ipc.c +++ b/drivers/platform/x86/intel_pmc_ipc.c @@ -68,8 +68,12 @@ #define PLAT_RESOURCE_IPC_INDEX 0 #define PLAT_RESOURCE_IPC_SIZE 0x1000 #define PLAT_RESOURCE_GCR_SIZE 0x1000 -#define PLAT_RESOURCE_PUNIT_DATA_INDEX 1 -#define PLAT_RESOURCE_PUNIT_INTER_INDEX 2 +#define PLAT_RESOURCE_BIOS_DATA_INDEX 1 +#define PLAT_RESOURCE_BIOS_INTER_INDEX 2 +#define PLAT_RESOURCE_ISP_DATA_INDEX 4 +#define PLAT_RESOURCE_ISP_INTER_INDEX 5 +#define PLAT_RESOURCE_GTD_DATA_INDEX 6 +#define PLAT_RESOURCE_GTD_INTER_INDEX 7 #define PLAT_RESOURCE_ACPI_IO_INDEX 0 /* @@ -105,10 +109,6 @@ static struct intel_pmc_ipc_dev { int gcr_size; /* punit */ - resource_size_t punit_base; - int punit_size; - resource_size_t punit_base2; - int punit_size2; struct platform_device *punit_dev; } ipcdev; @@ -444,9 +444,22 @@ static const struct attribute_group intel_ipc_group = { .attrs = intel_ipc_attrs, }; -#define PUNIT_RESOURCE_INTER 1 -static struct resource punit_res[] = { - /* Punit */ +static struct resource punit_res_array[] = { + /* Punit BIOS */ + { + .flags = IORESOURCE_MEM, + }, + { + .flags = IORESOURCE_MEM, + }, + /* Punit ISP */ + { + .flags = IORESOURCE_MEM, + }, + { + .flags = IORESOURCE_MEM, + }, + /* Punit GTD */ { .flags = IORESOURCE_MEM, }, @@ -481,7 +494,6 @@ static struct itco_wdt_platform_data tco_info = { static int ipc_create_punit_device(void) { struct platform_device *pdev; - struct resource *res; int ret; pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1); @@ -491,17 +503,8 @@ static int ipc_create_punit_device(void) } pdev->dev.parent = ipcdev.dev; - - res = punit_res; - res->start = ipcdev.punit_base; - res->end = res->start + ipcdev.punit_size - 1; - - res = punit_res + PUNIT_RESOURCE_INTER; - res->start = ipcdev.punit_base2; - res->end = res->start + ipcdev.punit_size2 - 1; - - ret = platform_device_add_resources(pdev, punit_res, - ARRAY_SIZE(punit_res)); + ret = platform_device_add_resources(pdev, punit_res_array, + ARRAY_SIZE(punit_res_array)); if (ret) { dev_err(ipcdev.dev, "Failed to add platform punit resources\n"); goto err; @@ -590,7 +593,7 @@ static int ipc_create_pmc_devices(void) static int ipc_plat_get_res(struct platform_device *pdev) { - struct resource *res; + struct resource *res, *punit_res; void __iomem *addr; int size; @@ -606,28 +609,71 @@ static int ipc_plat_get_res(struct platform_device *pdev) dev_info(&pdev->dev, "io res: %llx %x\n", (long long)res->start, (int)resource_size(res)); + /* This is index 0 to cover BIOS data register */ + punit_res = punit_res_array; res = platform_get_resource(pdev, IORESOURCE_MEM, - PLAT_RESOURCE_PUNIT_DATA_INDEX); + PLAT_RESOURCE_BIOS_DATA_INDEX); if (!res) { - dev_err(&pdev->dev, "Failed to get punit resource\n"); + dev_err(&pdev->dev, "Failed to get res of punit BIOS data\n"); return -ENXIO; } - size = resource_size(res); - ipcdev.punit_base = res->start; - ipcdev.punit_size = size; - dev_info(&pdev->dev, "punit data res: %llx %x\n", + *punit_res = *res; + dev_info(&pdev->dev, "punit BIOS data res: %llx %x\n", (long long)res->start, (int)resource_size(res)); res = platform_get_resource(pdev, IORESOURCE_MEM, - PLAT_RESOURCE_PUNIT_INTER_INDEX); + PLAT_RESOURCE_BIOS_INTER_INDEX); if (!res) { - dev_err(&pdev->dev, "Failed to get punit inter resource\n"); + dev_err(&pdev->dev, "Failed to get res of punit BIOS inter\n"); return -ENXIO; } - size = resource_size(res); - ipcdev.punit_base2 = res->start; - ipcdev.punit_size2 = size; - dev_info(&pdev->dev, "punit interface res: %llx %x\n", + /* This is index 1 to cover BIOS interface register */ + *++punit_res = *res; + dev_info(&pdev->dev, "punit BIOS interface res: %llx %x\n", + (long long)res->start, (int)resource_size(res)); + + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_ISP_DATA_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get res of punit ISP data\n"); + return -ENXIO; + } + /* This is index 2 to cover ISP data register */ + *++punit_res = *res; + dev_info(&pdev->dev, "punit ISP data res: %llx %x\n", + (long long)res->start, (int)resource_size(res)); + + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_ISP_INTER_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get res of punit ISP inter\n"); + return -ENXIO; + } + /* This is index 3 to cover ISP interface register */ + *++punit_res = *res; + dev_info(&pdev->dev, "punit ISP interface res: %llx %x\n", + (long long)res->start, (int)resource_size(res)); + + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_GTD_DATA_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get res of punit GTD data\n"); + return -ENXIO; + } + /* This is index 4 to cover GTD data register */ + *++punit_res = *res; + dev_info(&pdev->dev, "punit GTD data res: %llx %x\n", + (long long)res->start, (int)resource_size(res)); + + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_GTD_INTER_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get res of punit GTD inter\n"); + return -ENXIO; + } + /* This is index 5 to cover GTD interface register */ + *++punit_res = *res; + dev_info(&pdev->dev, "punit GTD interface res: %llx %x\n", (long long)res->start, (int)resource_size(res)); res = platform_get_resource(pdev, IORESOURCE_MEM,
BIOS restructure exported memory resources for Punit in acpi table, So update resources for Punit. Signed-off-by: Qipeng Zha <qipeng.zha@intel.com> --- change in v9 rename indexes of all acpi resources; add comments for punit_res; --- drivers/platform/x86/intel_pmc_ipc.c | 114 ++++++++++++++++++++++++----------- 1 file changed, 80 insertions(+), 34 deletions(-)