From: Niklas Cassel <niklas.cassel@axis.com>
To: Kishon Vijay Abraham I <kishon@ti.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 13:31:27 +0100 [thread overview]
Message-ID: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com> (raw)
In-Reply-To: <58C28FCC.4070100@ti.com>
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.
For dbi_base2, you've added error checking in platform specific code
(pci-dra7xx.c), but I don't see any error checking in your patch proposal
pcie-designware-ep.c:dw_pcie_ep_init.
If you add error checking in dw_pcie_ep_init, then I agree,
we don't need any error checking in dw_pcie_writel_dbi2.
>
> Thanks
> Kishon
WARNING: multiple messages have this Message-ID (diff)
From: Niklas Cassel <niklas.cassel@axis.com>
To: Kishon Vijay Abraham I <kishon@ti.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 13:31:27 +0100 [thread overview]
Message-ID: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com> (raw)
In-Reply-To: <58C28FCC.4070100@ti.com>
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.
For dbi_base2, you've added error checking in platform specific code
(pci-dra7xx.c), but I don't see any error checking in your patch proposal
pcie-designware-ep.c:dw_pcie_ep_init.
If you add error checking in dw_pcie_ep_init, then I agree,
we don't need any error checking in dw_pcie_writel_dbi2.
>
> 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: Niklas Cassel <niklas.cassel@axis.com>
To: Kishon Vijay Abraham I <kishon@ti.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 13:31:27 +0100 [thread overview]
Message-ID: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com> (raw)
In-Reply-To: <58C28FCC.4070100@ti.com>
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.
For dbi_base2, you've added error checking in platform specific code
(pci-dra7xx.c), but I don't see any error checking in your patch proposal
pcie-designware-ep.c:dw_pcie_ep_init.
If you add error checking in dw_pcie_ep_init, then I agree,
we don't need any error checking in dw_pcie_writel_dbi2.
>
> Thanks
> Kishon
WARNING: multiple messages have this Message-ID (diff)
From: niklas.cassel@axis.com (Niklas Cassel)
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 13:31:27 +0100 [thread overview]
Message-ID: <d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com> (raw)
In-Reply-To: <58C28FCC.4070100@ti.com>
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.
For dbi_base2, you've added error checking in platform specific code
(pci-dra7xx.c), but I don't see any error checking in your patch proposal
pcie-designware-ep.c:dw_pcie_ep_init.
If you add error checking in dw_pcie_ep_init, then I agree,
we don't need any error checking in dw_pcie_writel_dbi2.
>
> Thanks
> Kishon
next prev parent reply other threads:[~2017-03-10 12:31 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 [this message]
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
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=d920d892-af48-b94f-6eb7-e9e0ff5d40c0@axis.com \
--to=niklas.cassel@axis.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=kishon@ti.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=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.