Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1283809 > unrolled thread

Re: [PATCH 3/9] ACPI: add struct acpi_amba_quirk for AMD pl330 specific device config

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/9] ACPI: add struct acpi_amba_quirk for AMD pl330  specific device config Graeme Gregory <gg@slimlogic.co.uk> - 2015-12-04 14:20 +0100
    RE: [PATCH 3/9] ACPI: add struct acpi_amba_quirk for AMD pl330  specific device config "Wang, Annie" <Annie.Wang@amd.com> - 2015-12-11 08:00 +0100
      Re: [PATCH 3/9] ACPI: add struct acpi_amba_quirk for AMD pl330  specific device config Vinod Koul <vinod.koul@intel.com> - 2015-12-11 10:30 +0100

#1283809 — 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1289261

>-----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 linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289354

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

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web