From: Kishon Vijay Abraham I <kishon@ti.com>
To: Niklas Cassel <niklas.cassel@axis.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Joao Pinto <Joao.Pinto@synopsys.com>, <linux-pci@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>
Cc: <nsekhar@ti.com>, Jingoo Han <jingoohan1@gmail.com>,
Richard Zhu <hongxing.zhu@nxp.com>,
Lucas Stach <l.stach@pengutronix.de>,
Murali Karicheri <m-karicheri2@ti.com>,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
Jesper Nilsson <jespern@axis.com>,
Zhou Wang <wangzhou1@hisilicon.com>,
Gabriele Paoloni <gabriele.paoloni@huawei.com>
Subject: Re: [RESEND PATCH v3 4/7] PCI: dwc: all: Modify dbi accessors to take dbi_base as argument
Date: Fri, 10 Mar 2017 18:26:44 +0530 [thread overview]
Message-ID: <58C2A28C.60703@ti.com> (raw)
In-Reply-To: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com>
Hi Niklas,
On Friday 10 March 2017 06:01 PM, Niklas Cassel wrote:
> On 03/10/2017 12:36 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 09 March 2017 08:35 PM, Niklas Cassel wrote:
>>>
>>> On 03/09/2017 03:48 PM, Niklas Cassel wrote:
>>>> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>>>>> dwc has 2 dbi address space labeled dbics and dbics2. The existing
>>>>> helper to access dbi address space can access only dbics. However
>>>>> dbics2 has to be accessed for programming the BAR registers in the
>>>>> case of EP mode. This is in preparation for adding EP mode support
>>>>> to dwc driver.
>>>> Hello Kishon
>>>>
>>>> I don't really like the idea of adding an extra argument to every existing read/write.
>>>> Will not a read/write using dbi2 be quite uncommon compared to a read/write
>>>> using dbi?
>>>>
>>>> How about something like this:
>>>>
>>>> void __dw_pcie_writel(struct dw_pcie *pci, void __iomem *base, u32 reg, u32 val)
>>>> {
>>>> if (pci->ops->writel_dbi)
>>>> pci->ops->writel_dbi(pci, base, reg, val);
>>>> else
>>>> writel(val, base + reg);
>>>> }
>>>>
>>>> #define dw_pcie_writel_dbi(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base, reg, val)
>>>> #define dw_pcie_writel_dbi2(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base2, reg, val)
>>> Perhaps make dw_pcie_writel_dbi2 a function rather than a define,
>>> so we can return an error if pci->dbi_base2 == NULL.
>> Should we return an error? We don't return error for dbi_base either. I think
>> it should be sufficient to return errors while populating dbi_base or
>> dbi_base2. Otherwise it's a bug and should result in abort. Joao?
>
> Sorry for previous empty email.
>
>
> What I meant to write:
>
> Right now we do error checking for dbi_base in platform specific code
> and in pcie-designware-host.c:dw_pcie_host_init.
it's been done in dw_pcie_host_init not as an error checking but since it's
*optional* for certain platforms to populate dbi_base (i.e where dbi_base is
mapped to configuration space), host_init takes care of assigning dbi_base to
configuration space address.
Thanks
Kishon
WARNING: multiple messages have this Message-ID (diff)
From: Kishon Vijay Abraham I <kishon@ti.com>
To: Niklas Cassel <niklas.cassel@axis.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Joao Pinto <Joao.Pinto@synopsys.com>, <linux-pci@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
Richard Zhu <hongxing.zhu@nxp.com>,
Gabriele Paoloni <gabriele.paoloni@huawei.com>,
Jingoo Han <jingoohan1@gmail.com>,
nsekhar@ti.com, Jesper Nilsson <jespern@axis.com>,
Zhou Wang <wangzhou1@hisilicon.com>,
Murali Karicheri <m-karicheri2@ti.com>,
Lucas Stach <l.stach@pengutronix.de>
Subject: Re: [RESEND PATCH v3 4/7] PCI: dwc: all: Modify dbi accessors to take dbi_base as argument
Date: Fri, 10 Mar 2017 18:26:44 +0530 [thread overview]
Message-ID: <58C2A28C.60703@ti.com> (raw)
In-Reply-To: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com>
Hi Niklas,
On Friday 10 March 2017 06:01 PM, Niklas Cassel wrote:
> On 03/10/2017 12:36 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 09 March 2017 08:35 PM, Niklas Cassel wrote:
>>>
>>> On 03/09/2017 03:48 PM, Niklas Cassel wrote:
>>>> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>>>>> dwc has 2 dbi address space labeled dbics and dbics2. The existing
>>>>> helper to access dbi address space can access only dbics. However
>>>>> dbics2 has to be accessed for programming the BAR registers in the
>>>>> case of EP mode. This is in preparation for adding EP mode support
>>>>> to dwc driver.
>>>> Hello Kishon
>>>>
>>>> I don't really like the idea of adding an extra argument to every existing read/write.
>>>> Will not a read/write using dbi2 be quite uncommon compared to a read/write
>>>> using dbi?
>>>>
>>>> How about something like this:
>>>>
>>>> void __dw_pcie_writel(struct dw_pcie *pci, void __iomem *base, u32 reg, u32 val)
>>>> {
>>>> if (pci->ops->writel_dbi)
>>>> pci->ops->writel_dbi(pci, base, reg, val);
>>>> else
>>>> writel(val, base + reg);
>>>> }
>>>>
>>>> #define dw_pcie_writel_dbi(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base, reg, val)
>>>> #define dw_pcie_writel_dbi2(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base2, reg, val)
>>> Perhaps make dw_pcie_writel_dbi2 a function rather than a define,
>>> so we can return an error if pci->dbi_base2 == NULL.
>> Should we return an error? We don't return error for dbi_base either. I think
>> it should be sufficient to return errors while populating dbi_base or
>> dbi_base2. Otherwise it's a bug and should result in abort. Joao?
>
> Sorry for previous empty email.
>
>
> What I meant to write:
>
> Right now we do error checking for dbi_base in platform specific code
> and in pcie-designware-host.c:dw_pcie_host_init.
it's been done in dw_pcie_host_init not as an error checking but since it's
*optional* for certain platforms to populate dbi_base (i.e where dbi_base is
mapped to configuration space), host_init takes care of assigning dbi_base to
configuration space address.
Thanks
Kishon
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Kishon Vijay Abraham I <kishon@ti.com>
To: Niklas Cassel <niklas.cassel@axis.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Joao Pinto <Joao.Pinto@synopsys.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
Richard Zhu <hongxing.zhu@nxp.com>,
Gabriele Paoloni <gabriele.paoloni@huawei.com>,
Jingoo Han <jingoohan1@gmail.com>,
nsekhar@ti.com, Jesper Nilsson <jespern@axis.com>,
Zhou Wang <wangzhou1@hisilicon.com>,
Murali Karicheri <m-karicheri2@ti.com>,
Lucas Stach <l.stach@pengutronix.de>
Subject: Re: [RESEND PATCH v3 4/7] PCI: dwc: all: Modify dbi accessors to take dbi_base as argument
Date: Fri, 10 Mar 2017 18:26:44 +0530 [thread overview]
Message-ID: <58C2A28C.60703@ti.com> (raw)
In-Reply-To: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com>
Hi Niklas,
On Friday 10 March 2017 06:01 PM, Niklas Cassel wrote:
> On 03/10/2017 12:36 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 09 March 2017 08:35 PM, Niklas Cassel wrote:
>>>
>>> On 03/09/2017 03:48 PM, Niklas Cassel wrote:
>>>> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>>>>> dwc has 2 dbi address space labeled dbics and dbics2. The existing
>>>>> helper to access dbi address space can access only dbics. However
>>>>> dbics2 has to be accessed for programming the BAR registers in the
>>>>> case of EP mode. This is in preparation for adding EP mode support
>>>>> to dwc driver.
>>>> Hello Kishon
>>>>
>>>> I don't really like the idea of adding an extra argument to every existing read/write.
>>>> Will not a read/write using dbi2 be quite uncommon compared to a read/write
>>>> using dbi?
>>>>
>>>> How about something like this:
>>>>
>>>> void __dw_pcie_writel(struct dw_pcie *pci, void __iomem *base, u32 reg, u32 val)
>>>> {
>>>> if (pci->ops->writel_dbi)
>>>> pci->ops->writel_dbi(pci, base, reg, val);
>>>> else
>>>> writel(val, base + reg);
>>>> }
>>>>
>>>> #define dw_pcie_writel_dbi(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base, reg, val)
>>>> #define dw_pcie_writel_dbi2(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base2, reg, val)
>>> Perhaps make dw_pcie_writel_dbi2 a function rather than a define,
>>> so we can return an error if pci->dbi_base2 == NULL.
>> Should we return an error? We don't return error for dbi_base either. I think
>> it should be sufficient to return errors while populating dbi_base or
>> dbi_base2. Otherwise it's a bug and should result in abort. Joao?
>
> Sorry for previous empty email.
>
>
> What I meant to write:
>
> Right now we do error checking for dbi_base in platform specific code
> and in pcie-designware-host.c:dw_pcie_host_init.
it's been done in dw_pcie_host_init not as an error checking but since it's
*optional* for certain platforms to populate dbi_base (i.e where dbi_base is
mapped to configuration space), host_init takes care of assigning dbi_base to
configuration space address.
Thanks
Kishon
WARNING: multiple messages have this Message-ID (diff)
From: kishon@ti.com (Kishon Vijay Abraham I)
To: linux-arm-kernel@lists.infradead.org
Subject: [RESEND PATCH v3 4/7] PCI: dwc: all: Modify dbi accessors to take dbi_base as argument
Date: Fri, 10 Mar 2017 18:26:44 +0530 [thread overview]
Message-ID: <58C2A28C.60703@ti.com> (raw)
In-Reply-To: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com>
Hi Niklas,
On Friday 10 March 2017 06:01 PM, Niklas Cassel wrote:
> On 03/10/2017 12:36 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 09 March 2017 08:35 PM, Niklas Cassel wrote:
>>>
>>> On 03/09/2017 03:48 PM, Niklas Cassel wrote:
>>>> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>>>>> dwc has 2 dbi address space labeled dbics and dbics2. The existing
>>>>> helper to access dbi address space can access only dbics. However
>>>>> dbics2 has to be accessed for programming the BAR registers in the
>>>>> case of EP mode. This is in preparation for adding EP mode support
>>>>> to dwc driver.
>>>> Hello Kishon
>>>>
>>>> I don't really like the idea of adding an extra argument to every existing read/write.
>>>> Will not a read/write using dbi2 be quite uncommon compared to a read/write
>>>> using dbi?
>>>>
>>>> How about something like this:
>>>>
>>>> void __dw_pcie_writel(struct dw_pcie *pci, void __iomem *base, u32 reg, u32 val)
>>>> {
>>>> if (pci->ops->writel_dbi)
>>>> pci->ops->writel_dbi(pci, base, reg, val);
>>>> else
>>>> writel(val, base + reg);
>>>> }
>>>>
>>>> #define dw_pcie_writel_dbi(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base, reg, val)
>>>> #define dw_pcie_writel_dbi2(pci, reg, val) __dw_pcie_writel(pci, pci->dbi_base2, reg, val)
>>> Perhaps make dw_pcie_writel_dbi2 a function rather than a define,
>>> so we can return an error if pci->dbi_base2 == NULL.
>> Should we return an error? We don't return error for dbi_base either. I think
>> it should be sufficient to return errors while populating dbi_base or
>> dbi_base2. Otherwise it's a bug and should result in abort. Joao?
>
> Sorry for previous empty email.
>
>
> What I meant to write:
>
> Right now we do error checking for dbi_base in platform specific code
> and in pcie-designware-host.c:dw_pcie_host_init.
it's been done in dw_pcie_host_init not as an error checking but since it's
*optional* for certain platforms to populate dbi_base (i.e where dbi_base is
mapped to configuration space), host_init takes care of assigning dbi_base to
configuration space address.
Thanks
Kishon
next prev parent reply other threads:[~2017-03-10 13:00 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-09 6:38 [RESEND PATCH v3 0/7] PCI: dwc: Miscellaneous fixes and cleanups Kishon Vijay Abraham I
2017-03-09 6:38 ` Kishon Vijay Abraham I
2017-03-09 6:38 ` Kishon Vijay Abraham I
2017-03-09 6:38 ` Kishon Vijay Abraham I
2017-03-09 6:38 ` [RESEND PATCH v3 1/7] PCI: dwc: designware: Add new *ops* for cpu addr fixup Kishon Vijay Abraham I
2017-03-09 6:38 ` Kishon Vijay Abraham I
2017-03-09 6:38 ` Kishon Vijay Abraham I
2017-03-09 6:38 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` [RESEND PATCH v3 2/7] PCI: dwc: dra7xx: Populate cpu_addr_fixup ops Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` [RESEND PATCH v3 3/7] PCI: dwc: artpec6: " Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 10:21 ` Niklas Cassel
2017-03-09 10:21 ` Niklas Cassel
2017-03-09 10:21 ` Niklas Cassel
2017-03-09 10:21 ` Niklas Cassel
2017-03-09 6:39 ` [RESEND PATCH v3 4/7] PCI: dwc: all: Modify dbi accessors to take dbi_base as argument Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 14:48 ` Niklas Cassel
2017-03-09 14:48 ` Niklas Cassel
2017-03-09 14:48 ` Niklas Cassel
2017-03-09 14:48 ` Niklas Cassel
2017-03-09 15:05 ` Niklas Cassel
2017-03-09 15:05 ` Niklas Cassel
2017-03-09 15:05 ` Niklas Cassel
2017-03-09 15:05 ` Niklas Cassel
2017-03-10 11:36 ` Kishon Vijay Abraham I
2017-03-10 11:36 ` Kishon Vijay Abraham I
2017-03-10 11:36 ` Kishon Vijay Abraham I
2017-03-10 11:36 ` Kishon Vijay Abraham I
2017-03-10 12:23 ` Niklas Cassel
2017-03-10 12:23 ` Niklas Cassel
2017-03-10 12:23 ` Niklas Cassel
2017-03-10 12:23 ` Niklas Cassel
2017-03-10 12:30 ` Joao Pinto
2017-03-10 12:30 ` Joao Pinto
2017-03-10 12:30 ` Joao Pinto
2017-03-10 12:30 ` Joao Pinto
2017-03-10 12:31 ` Niklas Cassel
2017-03-10 12:31 ` Niklas Cassel
2017-03-10 12:31 ` Niklas Cassel
2017-03-10 12:31 ` Niklas Cassel
2017-03-10 12:56 ` Kishon Vijay Abraham I [this message]
2017-03-10 12:56 ` Kishon Vijay Abraham I
2017-03-10 12:56 ` Kishon Vijay Abraham I
2017-03-10 12:56 ` Kishon Vijay Abraham I
2017-03-10 15:47 ` Niklas Cassel
2017-03-10 15:47 ` Niklas Cassel
2017-03-10 15:47 ` Niklas Cassel
2017-03-10 15:47 ` Niklas Cassel
2017-03-13 5:30 ` Kishon Vijay Abraham I
2017-03-13 5:30 ` Kishon Vijay Abraham I
2017-03-13 5:30 ` Kishon Vijay Abraham I
2017-03-13 5:30 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` [RESEND PATCH v3 5/7] PCI: dwc: all: Modify dbi accessors to access data of 4/2/1 bytes Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 14:48 ` Niklas Cassel
2017-03-09 14:48 ` Niklas Cassel
2017-03-09 14:48 ` Niklas Cassel
2017-03-09 14:48 ` Niklas Cassel
2017-03-10 12:04 ` Kishon Vijay Abraham I
2017-03-10 12:04 ` Kishon Vijay Abraham I
2017-03-10 12:04 ` Kishon Vijay Abraham I
2017-03-10 12:04 ` Kishon Vijay Abraham I
2017-03-10 12:56 ` Niklas Cassel
2017-03-10 12:56 ` Niklas Cassel
2017-03-10 12:56 ` Niklas Cassel
2017-03-10 12:56 ` Niklas Cassel
2017-03-10 13:04 ` Kishon Vijay Abraham I
2017-03-10 13:04 ` Kishon Vijay Abraham I
2017-03-10 13:04 ` Kishon Vijay Abraham I
2017-03-10 13:04 ` Kishon Vijay Abraham I
2017-03-10 14:59 ` Niklas Cassel
2017-03-10 14:59 ` Niklas Cassel
2017-03-10 14:59 ` Niklas Cassel
2017-03-10 14:59 ` Niklas Cassel
2017-03-09 6:39 ` [RESEND PATCH v3 6/7] PCI: dwc: designware: Move _unroll configurations to a separate function Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 12:25 ` Joao Pinto
2017-03-09 12:25 ` Joao Pinto
2017-03-09 12:25 ` Joao Pinto
2017-03-09 12:25 ` Joao Pinto
2017-03-09 6:39 ` [RESEND PATCH v3 7/7] PCI: dwc: dra7xx: Push request_irq call to the bottom of probe Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
2017-03-09 6:39 ` Kishon Vijay Abraham I
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=58C2A28C.60703@ti.com \
--to=kishon@ti.com \
--cc=Joao.Pinto@synopsys.com \
--cc=bhelgaas@google.com \
--cc=gabriele.paoloni@huawei.com \
--cc=hongxing.zhu@nxp.com \
--cc=jespern@axis.com \
--cc=jingoohan1@gmail.com \
--cc=l.stach@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=m-karicheri2@ti.com \
--cc=niklas.cassel@axis.com \
--cc=nsekhar@ti.com \
--cc=thomas.petazzoni@free-electrons.com \
--cc=wangzhou1@hisilicon.com \
/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.