All of lore.kernel.org
 help / color / mirror / Atom feed
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: keystone: add bus notifier to set dma_pfn_offset for pci devices
Date: Fri, 10 Oct 2014 20:04:57 -0400	[thread overview]
Message-ID: <54387429.2030605@ti.com> (raw)
In-Reply-To: <54385187.6030002@ti.com>



On 10/10/14 5:37 PM, Murali Karicheri wrote:
> On 10/10/2014 02:22 PM, Arnd Bergmann wrote:
>> On Friday 10 October 2014 11:15:37 Murali Karicheri wrote:
>>> @@ -54,6 +55,8 @@ static void __init keystone_init(void)
>>>          keystone_pm_runtime_init();
>>>          if (platform_nb.notifier_call)
>>>                  bus_register_notifier(&platform_bus_type,&platform_nb);
>>> +       if (platform_nb.notifier_call)
>>> +               bus_register_notifier(&pci_bus_type,&platform_nb);
>>>          of_platform_populate(NULL, of_default_bus_match_table, NULL,
>>> NULL);
>>>
>>
>> No, this looks very wrong. Santosh spent an enormous effort on obsoleting
>> the platform notifier block by adding the range parser to the platform
>> device probe path.
>>
>> You should really remove platform_nb and all associated code rather than
>> adding more code to it.
>>
>> NAK
>>
>>     Arnd
> Arnd,
>
> I took a look at the recent work from Santosh where he has introduced
> dma-range property to set the dma_pfn_offset for platform devices. Are
> you referring to this commit below?
>
Its the correct commit.

> commit 591c1ee465ce5372385dbc41e7d3e36cbb477bd8
> Author: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Date:   Thu Apr 24 11:30:04 2014 -0400
>
>      of: configure the platform device dma parameters
>
>      Retrieve DMA configuration from DT and setup platform device's DMA
>      parameters. The DMA configuration in DT has to be specified using
>      "dma-ranges" and "dma-coherent" properties if supported.
>
>      We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops
>      using "dma-coherent" device tree properties.
>
>      The set_arch_dma_coherent_ops macro has to be defined by arch if
>      it supports coherent dma_ops. Otherwise,
> set_arch_dma_coherent_ops() is
>      declared as nop.
>
> Based on this, dma configuration parameters get set for the device which
> is probed through DT.
>
> As PCI devices are attached to the PCI bus during scan, and we don't
> have DT nodes, we could use similar mechanism to pass the dma-range info
> from parent host platform device to the PCI devices by adding an
> of_pci_dma_configure() API and hook it to the PCI probe path some where?
> Please comment on this so that I can work on the right solution to
> address this issue for Keystone.
>
Adding the DT node parsing code in PCI bus probe path is the right way
to go about it. You could re-use some of the helpers from dma parsing
code.

I let Arnd comment if he disagrees, otherwise I suggest to create an
RFC patch and post it on the list. We can take it from there.

That also reminded me xhci host code issue with dma-ranges since the
devices are manually created there. I will review that thread as
well after this merge window.

Regards,
Santosh

WARNING: multiple messages have this Message-ID (diff)
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Murali Karicheri <m-karicheri2@ti.com>, Arnd Bergmann <arnd@arndb.de>
Cc: <linux-arm-kernel@lists.infradead.org>, <linux@arm.linux.org.uk>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ARM: keystone: add bus notifier to set dma_pfn_offset for pci devices
Date: Fri, 10 Oct 2014 20:04:57 -0400	[thread overview]
Message-ID: <54387429.2030605@ti.com> (raw)
In-Reply-To: <54385187.6030002@ti.com>



On 10/10/14 5:37 PM, Murali Karicheri wrote:
> On 10/10/2014 02:22 PM, Arnd Bergmann wrote:
>> On Friday 10 October 2014 11:15:37 Murali Karicheri wrote:
>>> @@ -54,6 +55,8 @@ static void __init keystone_init(void)
>>>          keystone_pm_runtime_init();
>>>          if (platform_nb.notifier_call)
>>>                  bus_register_notifier(&platform_bus_type,&platform_nb);
>>> +       if (platform_nb.notifier_call)
>>> +               bus_register_notifier(&pci_bus_type,&platform_nb);
>>>          of_platform_populate(NULL, of_default_bus_match_table, NULL,
>>> NULL);
>>>
>>
>> No, this looks very wrong. Santosh spent an enormous effort on obsoleting
>> the platform notifier block by adding the range parser to the platform
>> device probe path.
>>
>> You should really remove platform_nb and all associated code rather than
>> adding more code to it.
>>
>> NAK
>>
>>     Arnd
> Arnd,
>
> I took a look at the recent work from Santosh where he has introduced
> dma-range property to set the dma_pfn_offset for platform devices. Are
> you referring to this commit below?
>
Its the correct commit.

> commit 591c1ee465ce5372385dbc41e7d3e36cbb477bd8
> Author: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Date:   Thu Apr 24 11:30:04 2014 -0400
>
>      of: configure the platform device dma parameters
>
>      Retrieve DMA configuration from DT and setup platform device's DMA
>      parameters. The DMA configuration in DT has to be specified using
>      "dma-ranges" and "dma-coherent" properties if supported.
>
>      We setup dma_pfn_offset using "dma-ranges" and dma_coherent_ops
>      using "dma-coherent" device tree properties.
>
>      The set_arch_dma_coherent_ops macro has to be defined by arch if
>      it supports coherent dma_ops. Otherwise,
> set_arch_dma_coherent_ops() is
>      declared as nop.
>
> Based on this, dma configuration parameters get set for the device which
> is probed through DT.
>
> As PCI devices are attached to the PCI bus during scan, and we don't
> have DT nodes, we could use similar mechanism to pass the dma-range info
> from parent host platform device to the PCI devices by adding an
> of_pci_dma_configure() API and hook it to the PCI probe path some where?
> Please comment on this so that I can work on the right solution to
> address this issue for Keystone.
>
Adding the DT node parsing code in PCI bus probe path is the right way
to go about it. You could re-use some of the helpers from dma parsing
code.

I let Arnd comment if he disagrees, otherwise I suggest to create an
RFC patch and post it on the list. We can take it from there.

That also reminded me xhci host code issue with dma-ranges since the
devices are manually created there. I will review that thread as
well after this merge window.

Regards,
Santosh

  reply	other threads:[~2014-10-11  0:04 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-10 15:15 [PATCH] ARM: keystone: add bus notifier to set dma_pfn_offset for pci devices Murali Karicheri
2014-10-10 15:15 ` Murali Karicheri
2014-10-10 15:29 ` Santosh Shilimkar
2014-10-10 15:29   ` Santosh Shilimkar
2014-10-10 15:42   ` Russell King - ARM Linux
2014-10-10 15:42     ` Russell King - ARM Linux
2014-10-10 15:46     ` Santosh Shilimkar
2014-10-10 15:46       ` Santosh Shilimkar
2014-10-10 15:53     ` Murali Karicheri
2014-10-10 15:53       ` Murali Karicheri
2014-10-10 18:22 ` Arnd Bergmann
2014-10-10 18:22   ` Arnd Bergmann
2014-10-10 21:37   ` Murali Karicheri
2014-10-10 21:37     ` Murali Karicheri
2014-10-11  0:04     ` Santosh Shilimkar [this message]
2014-10-11  0:04       ` Santosh Shilimkar
2014-10-11 20:37       ` Arnd Bergmann
2014-10-11 20:37         ` Arnd Bergmann
2014-10-13 14:13         ` Murali Karicheri
2014-10-13 14:13           ` Murali Karicheri

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54387429.2030605@ti.com \
    --to=santosh.shilimkar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.