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 5/7] PCI: dwc: all: Modify dbi accessors to access data of 4/2/1 bytes
Date: Fri, 10 Mar 2017 17:34:24 +0530 [thread overview]
Message-ID: <58C29648.5020708@ti.com> (raw)
In-Reply-To: <a56e462d-83ee-194f-1f9f-3f4c339cad46@axis.com>
Hi,
On Thursday 09 March 2017 08:18 PM, Niklas Cassel wrote:
> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>> Previously dbi accessors can be used to access data of size 4
>> bytes. But there might be situations (like accessing
>> MSI_MESSAGE_CONTROL in order to set/get the number of required
>> MSI interrupts in EP mode) where dbi accessors must
>> be used to access data of size 2. This is in preparation for
>> adding endpoint mode support to designware 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 of length != 32 be quite uncommon compared to
> a read/write of length == 32?
>
> How about adding some defines to pcie-designware.h:
>
> #define dw_pcie_writel_dbi(pci, base, reg, val) dw_pcie_write_dbi(pci, base, reg, 0x4, val)
> #define dw_pcie_readl_dbi(pci, base, reg) dw_pcie_read_dbi(pci, base, reg, 0x4)
>
> That way we don't have to change every existing read/write.
>
>
>
> Is there a reason why we can't just do:
>
> vial = dw_pcie_readl_dbi(pci, base, MSI_MESSAGE_CONTROL);
MSI_MESSAGE_CONTROL is 0x52 (MSI capability offset + 2). I'm not sure if we can
do a readl that crosses the alignment boundary in all platforms. The other
option is to readl from "MSI capability offset + 0" and extract the last 16
bits. I felt this is more clear since we are interested only in the
MSI_MESSAGE_CONTROL.
> <shifting+masking the bits we need to get/set>
> dw_pcie_writel_dbi(pci, base, MSI_MESSAGE_CONTROL, val);
>
> Or are we going to be doing read/writes of length != 32 so often that
> you think that it's cleaner to have this abstraction?
it's used mainly for accessing configuration space header fields. Even the pci
core uses *pci_read_config_word* for accessing such fields.
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 5/7] PCI: dwc: all: Modify dbi accessors to access data of 4/2/1 bytes
Date: Fri, 10 Mar 2017 17:34:24 +0530 [thread overview]
Message-ID: <58C29648.5020708@ti.com> (raw)
In-Reply-To: <a56e462d-83ee-194f-1f9f-3f4c339cad46@axis.com>
Hi,
On Thursday 09 March 2017 08:18 PM, Niklas Cassel wrote:
> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>> Previously dbi accessors can be used to access data of size 4
>> bytes. But there might be situations (like accessing
>> MSI_MESSAGE_CONTROL in order to set/get the number of required
>> MSI interrupts in EP mode) where dbi accessors must
>> be used to access data of size 2. This is in preparation for
>> adding endpoint mode support to designware 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 of length != 32 be quite uncommon compared to
> a read/write of length == 32?
>
> How about adding some defines to pcie-designware.h:
>
> #define dw_pcie_writel_dbi(pci, base, reg, val) dw_pcie_write_dbi(pci, base, reg, 0x4, val)
> #define dw_pcie_readl_dbi(pci, base, reg) dw_pcie_read_dbi(pci, base, reg, 0x4)
>
> That way we don't have to change every existing read/write.
>
>
>
> Is there a reason why we can't just do:
>
> vial = dw_pcie_readl_dbi(pci, base, MSI_MESSAGE_CONTROL);
MSI_MESSAGE_CONTROL is 0x52 (MSI capability offset + 2). I'm not sure if we can
do a readl that crosses the alignment boundary in all platforms. The other
option is to readl from "MSI capability offset + 0" and extract the last 16
bits. I felt this is more clear since we are interested only in the
MSI_MESSAGE_CONTROL.
> <shifting+masking the bits we need to get/set>
> dw_pcie_writel_dbi(pci, base, MSI_MESSAGE_CONTROL, val);
>
> Or are we going to be doing read/writes of length != 32 so often that
> you think that it's cleaner to have this abstraction?
it's used mainly for accessing configuration space header fields. Even the pci
core uses *pci_read_config_word* for accessing such fields.
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 5/7] PCI: dwc: all: Modify dbi accessors to access data of 4/2/1 bytes
Date: Fri, 10 Mar 2017 17:34:24 +0530 [thread overview]
Message-ID: <58C29648.5020708@ti.com> (raw)
In-Reply-To: <a56e462d-83ee-194f-1f9f-3f4c339cad46@axis.com>
Hi,
On Thursday 09 March 2017 08:18 PM, Niklas Cassel wrote:
> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>> Previously dbi accessors can be used to access data of size 4
>> bytes. But there might be situations (like accessing
>> MSI_MESSAGE_CONTROL in order to set/get the number of required
>> MSI interrupts in EP mode) where dbi accessors must
>> be used to access data of size 2. This is in preparation for
>> adding endpoint mode support to designware 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 of length != 32 be quite uncommon compared to
> a read/write of length == 32?
>
> How about adding some defines to pcie-designware.h:
>
> #define dw_pcie_writel_dbi(pci, base, reg, val) dw_pcie_write_dbi(pci, base, reg, 0x4, val)
> #define dw_pcie_readl_dbi(pci, base, reg) dw_pcie_read_dbi(pci, base, reg, 0x4)
>
> That way we don't have to change every existing read/write.
>
>
>
> Is there a reason why we can't just do:
>
> vial = dw_pcie_readl_dbi(pci, base, MSI_MESSAGE_CONTROL);
MSI_MESSAGE_CONTROL is 0x52 (MSI capability offset + 2). I'm not sure if we can
do a readl that crosses the alignment boundary in all platforms. The other
option is to readl from "MSI capability offset + 0" and extract the last 16
bits. I felt this is more clear since we are interested only in the
MSI_MESSAGE_CONTROL.
> <shifting+masking the bits we need to get/set>
> dw_pcie_writel_dbi(pci, base, MSI_MESSAGE_CONTROL, val);
>
> Or are we going to be doing read/writes of length != 32 so often that
> you think that it's cleaner to have this abstraction?
it's used mainly for accessing configuration space header fields. Even the pci
core uses *pci_read_config_word* for accessing such fields.
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 5/7] PCI: dwc: all: Modify dbi accessors to access data of 4/2/1 bytes
Date: Fri, 10 Mar 2017 17:34:24 +0530 [thread overview]
Message-ID: <58C29648.5020708@ti.com> (raw)
In-Reply-To: <a56e462d-83ee-194f-1f9f-3f4c339cad46@axis.com>
Hi,
On Thursday 09 March 2017 08:18 PM, Niklas Cassel wrote:
> On 03/09/2017 07:39 AM, Kishon Vijay Abraham I wrote:
>> Previously dbi accessors can be used to access data of size 4
>> bytes. But there might be situations (like accessing
>> MSI_MESSAGE_CONTROL in order to set/get the number of required
>> MSI interrupts in EP mode) where dbi accessors must
>> be used to access data of size 2. This is in preparation for
>> adding endpoint mode support to designware 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 of length != 32 be quite uncommon compared to
> a read/write of length == 32?
>
> How about adding some defines to pcie-designware.h:
>
> #define dw_pcie_writel_dbi(pci, base, reg, val) dw_pcie_write_dbi(pci, base, reg, 0x4, val)
> #define dw_pcie_readl_dbi(pci, base, reg) dw_pcie_read_dbi(pci, base, reg, 0x4)
>
> That way we don't have to change every existing read/write.
>
>
>
> Is there a reason why we can't just do:
>
> vial = dw_pcie_readl_dbi(pci, base, MSI_MESSAGE_CONTROL);
MSI_MESSAGE_CONTROL is 0x52 (MSI capability offset + 2). I'm not sure if we can
do a readl that crosses the alignment boundary in all platforms. The other
option is to readl from "MSI capability offset + 0" and extract the last 16
bits. I felt this is more clear since we are interested only in the
MSI_MESSAGE_CONTROL.
> <shifting+masking the bits we need to get/set>
> dw_pcie_writel_dbi(pci, base, MSI_MESSAGE_CONTROL, val);
>
> Or are we going to be doing read/writes of length != 32 so often that
> you think that it's cleaner to have this abstraction?
it's used mainly for accessing configuration space header fields. Even the pci
core uses *pci_read_config_word* for accessing such fields.
Thanks
Kishon
next prev parent reply other threads:[~2017-03-10 12:05 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
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 [this message]
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=58C29648.5020708@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.