Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 01.12.14 23:53, Stuart Yoder wrote:
>
>
>> -Original Message-
>> From: Alexander Graf [mailto:ag...@suse.de]
>> Sent: Thursday, November 27, 2014 10:14 AM
>> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
>> linux-kernel@vger.kernel.org
>> Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
>> Bogdan-BHAMCIU1; Marginean
>> Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
>> Nir-RM30794; Schmitt Richard-B43082
>> Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
>> APIs
>>
>>
>>
>> On 13.11.14 18:54, J. German Rivera wrote:
>>> APIs to access the Management Complex (MC) hardware
>>> module of Freescale LS2 SoCs. This patch includes
>>> APIs to check the MC firmware version and to manipulate
>>> DPRC objects in the MC.
>>>
>>> Signed-off-by: J. German Rivera
>>> Signed-off-by: Stuart Yoder
>>
>> [...]
>>
>>> +/**
>>> + * Creates an MC I/O object
>>> + *
>>> + * @dev: device to be associated with the MC I/O object
>>> + * @mc_portal_phys_addr: physical address of the MC portal to use
>>> + * @mc_portal_size: size in bytes of the MC portal
>>> + * @flags: flags for the new MC I/O object
>>> + * @new_mc_io: Area to return pointer to newly created MC I/O object
>>> + *
>>> + * Returns '0' on Success; Error code otherwise.
>>> + */
>>> +int __must_check fsl_create_mc_io(struct device *dev,
>>> + phys_addr_t mc_portal_phys_addr,
>>> + uint32_t mc_portal_size,
>>> + uint32_t flags, struct fsl_mc_io **new_mc_io)
>>> +{
>>> + struct fsl_mc_io *mc_io;
>>> + void __iomem *mc_portal_virt_addr;
>>> + struct resource *res;
>>> +
>>> + mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
>>> + if (mc_io == NULL)
>>> + return -ENOMEM;
>>> +
>>> + mc_io->dev = dev;
>>> + mc_io->flags = flags;
>>> + mc_io->portal_phys_addr = mc_portal_phys_addr;
>>> + mc_io->portal_size = mc_portal_size;
>>> + res = devm_request_mem_region(dev,
>>> + mc_portal_phys_addr,
>>> + mc_portal_size,
>>> + "mc_portal");
>>> + if (res == NULL) {
>>> + dev_err(dev,
>>> + "devm_request_mem_region failed for MC portal %#llx\n",
>>> + mc_portal_phys_addr);
>>> + return -EBUSY;
>>> + }
>>> +
>>> + mc_portal_virt_addr = devm_ioremap_nocache(dev,
>>> + mc_portal_phys_addr,
>>> + mc_portal_size);
>>
>> While I can't complain about the device itself, I will note that I think
>> it's a pretty bad design decision to expose actual host physical
>> addresses in the protocol.
>
> I tend to agree. I'll look into creating a proposed change to the
> architecture
> to have the MC communicate a physical offset of some kind.
>
>> Basically this means that you won't be able to pass a full MC complex
>> into a guest, even if you could virtualize IRQ and DMA access unless you
>> map it at the exact same location as the host's MC complex.
>
> Right. But is that really an issue in practice?
Well, it obviously depends on what you're trying to do. For everything
that's envisioned today I don't think it's a problem, but I like to
stick to the "know as little as you have to know" rule when it comes to
communication protocols.
>> Could we at least add a "ranges" property to the MC device description
>> and check whether the physical addresses we get are within that range -
>> if nothing else, at least as sanity check? Then maybe add some calls in
>> the next version that act on that range rather than actual host physical
>> addresses?
>
> So you mean something like:
>
> fsl_mc: fsl-mc@80c00 {
> compatible = "fsl,qoriq-mc";
> #stream-id-cells = <2>;
> reg = <0x0008 0x0c00 0 0x40>,/* MC portal base */
> <0x 0x0834 0 0x4>; /* MC control reg */
> ranges = <0x8 0x0 0x8 0x0 0x2000>;
> lpi-parent = <>;
> };
>
> The physical addresses returned by the MC fall into a 512MB "portal"
> region at 0x8__ in the physical address map. For now map it 1:1, but
> in the
> future it could become:
>ranges = <0x0 0x0 0x8 0x0 0x2000>;
>
> ...if I can get the hardware architecture changed.
Yup, I think that makes things a lot less error prone - you don't
randomly access any pointer the device tells you to access :).
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 01.12.14 23:28, Stuart Yoder wrote:
>
>
>> -Original Message-
>> From: Alexander Graf [mailto:ag...@suse.de]
>> Sent: Thursday, November 27, 2014 8:30 AM
>> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
>> linux-kernel@vger.kernel.org
>> Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
>> Bogdan-BHAMCIU1; Marginean
>> Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
>> Nir-RM30794; Schmitt Richard-B43082
>> Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
>> APIs
>>
>>
>>
>> On 13.11.14 18:54, J. German Rivera wrote:
>>> APIs to access the Management Complex (MC) hardware
>>> module of Freescale LS2 SoCs. This patch includes
>>> APIs to check the MC firmware version and to manipulate
>>> DPRC objects in the MC.
>>>
>>> Signed-off-by: J. German Rivera
>>> Signed-off-by: Stuart Yoder
>>
>> [...]
>>
>>> +/*
>>> + * Object descriptor, returned from dprc_get_obj()
>>> + */
>>> +struct dprc_obj_desc {
>>> + /* Type of object: NULL terminated string */
>>> + char type[16];
>>
>> I don't see where it actually gets NULL terminated - all 16 bytes come
>> directly from the device.
>>
>> While it's probably ok to trust it, I think we'd still be safer off if
>> we just make this a char[17] array to always have our NULL terminating
>> string. That way we're guaranteed we'll never run over our memory
>> boundaries.
>
> The device is supposed to guarantee that the string is null
> terminated...so there will never be valid chars in the 16th
> character. So, what about just forcing type[15] = '\0'?
>
> I think that would be better than making it a char[17].
Sure, that works too.
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, November 27, 2014 10:14 AM
> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
> linux-kernel@vger.kernel.org
> Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
> Bogdan-BHAMCIU1; Marginean
> Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
> Nir-RM30794; Schmitt Richard-B43082
> Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
> APIs
>
>
>
> On 13.11.14 18:54, J. German Rivera wrote:
> > APIs to access the Management Complex (MC) hardware
> > module of Freescale LS2 SoCs. This patch includes
> > APIs to check the MC firmware version and to manipulate
> > DPRC objects in the MC.
> >
> > Signed-off-by: J. German Rivera
> > Signed-off-by: Stuart Yoder
>
> [...]
>
> > +/**
> > + * Creates an MC I/O object
> > + *
> > + * @dev: device to be associated with the MC I/O object
> > + * @mc_portal_phys_addr: physical address of the MC portal to use
> > + * @mc_portal_size: size in bytes of the MC portal
> > + * @flags: flags for the new MC I/O object
> > + * @new_mc_io: Area to return pointer to newly created MC I/O object
> > + *
> > + * Returns '0' on Success; Error code otherwise.
> > + */
> > +int __must_check fsl_create_mc_io(struct device *dev,
> > + phys_addr_t mc_portal_phys_addr,
> > + uint32_t mc_portal_size,
> > + uint32_t flags, struct fsl_mc_io **new_mc_io)
> > +{
> > + struct fsl_mc_io *mc_io;
> > + void __iomem *mc_portal_virt_addr;
> > + struct resource *res;
> > +
> > + mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
> > + if (mc_io == NULL)
> > + return -ENOMEM;
> > +
> > + mc_io->dev = dev;
> > + mc_io->flags = flags;
> > + mc_io->portal_phys_addr = mc_portal_phys_addr;
> > + mc_io->portal_size = mc_portal_size;
> > + res = devm_request_mem_region(dev,
> > + mc_portal_phys_addr,
> > + mc_portal_size,
> > + "mc_portal");
> > + if (res == NULL) {
> > + dev_err(dev,
> > + "devm_request_mem_region failed for MC portal %#llx\n",
> > + mc_portal_phys_addr);
> > + return -EBUSY;
> > + }
> > +
> > + mc_portal_virt_addr = devm_ioremap_nocache(dev,
> > + mc_portal_phys_addr,
> > + mc_portal_size);
>
> While I can't complain about the device itself, I will note that I think
> it's a pretty bad design decision to expose actual host physical
> addresses in the protocol.
I tend to agree. I'll look into creating a proposed change to the architecture
to have the MC communicate a physical offset of some kind.
> Basically this means that you won't be able to pass a full MC complex
> into a guest, even if you could virtualize IRQ and DMA access unless you
> map it at the exact same location as the host's MC complex.
Right. But is that really an issue in practice?
> Could we at least add a "ranges" property to the MC device description
> and check whether the physical addresses we get are within that range -
> if nothing else, at least as sanity check? Then maybe add some calls in
> the next version that act on that range rather than actual host physical
> addresses?
So you mean something like:
fsl_mc: fsl-mc@80c00 {
compatible = "fsl,qoriq-mc";
#stream-id-cells = <2>;
reg = <0x0008 0x0c00 0 0x40>,/* MC portal base */
<0x 0x0834 0 0x4>; /* MC control reg */
ranges = <0x8 0x0 0x8 0x0 0x2000>;
lpi-parent = <>;
};
The physical addresses returned by the MC fall into a 512MB "portal"
region at 0x8__ in the physical address map. For now map it 1:1, but
in the
future it could become:
ranges = <0x0 0x0 0x8 0x0 0x2000>;
...if I can get the hardware architecture changed.
Stuart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, November 27, 2014 8:30 AM
> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
> linux-kernel@vger.kernel.org
> Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
> Bogdan-BHAMCIU1; Marginean
> Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
> Nir-RM30794; Schmitt Richard-B43082
> Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
> APIs
>
>
>
> On 13.11.14 18:54, J. German Rivera wrote:
> > APIs to access the Management Complex (MC) hardware
> > module of Freescale LS2 SoCs. This patch includes
> > APIs to check the MC firmware version and to manipulate
> > DPRC objects in the MC.
> >
> > Signed-off-by: J. German Rivera
> > Signed-off-by: Stuart Yoder
>
> [...]
>
> > +/*
> > + * Object descriptor, returned from dprc_get_obj()
> > + */
> > +struct dprc_obj_desc {
> > + /* Type of object: NULL terminated string */
> > + char type[16];
>
> I don't see where it actually gets NULL terminated - all 16 bytes come
> directly from the device.
>
> While it's probably ok to trust it, I think we'd still be safer off if
> we just make this a char[17] array to always have our NULL terminating
> string. That way we're guaranteed we'll never run over our memory
> boundaries.
The device is supposed to guarantee that the string is null
terminated...so there will never be valid chars in the 16th
character. So, what about just forcing type[15] = '\0'?
I think that would be better than making it a char[17].
Stuart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, November 27, 2014 8:30 AM
To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
linux-kernel@vger.kernel.org
Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
Bogdan-BHAMCIU1; Marginean
Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
Nir-RM30794; Schmitt Richard-B43082
Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
APIs
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
[...]
+/*
+ * Object descriptor, returned from dprc_get_obj()
+ */
+struct dprc_obj_desc {
+ /* Type of object: NULL terminated string */
+ char type[16];
I don't see where it actually gets NULL terminated - all 16 bytes come
directly from the device.
While it's probably ok to trust it, I think we'd still be safer off if
we just make this a char[17] array to always have our NULL terminating
string. That way we're guaranteed we'll never run over our memory
boundaries.
The device is supposed to guarantee that the string is null
terminated...so there will never be valid chars in the 16th
character. So, what about just forcing type[15] = '\0'?
I think that would be better than making it a char[17].
Stuart
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, November 27, 2014 10:14 AM
To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
linux-kernel@vger.kernel.org
Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
Bogdan-BHAMCIU1; Marginean
Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
Nir-RM30794; Schmitt Richard-B43082
Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
APIs
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
[...]
+/**
+ * Creates an MC I/O object
+ *
+ * @dev: device to be associated with the MC I/O object
+ * @mc_portal_phys_addr: physical address of the MC portal to use
+ * @mc_portal_size: size in bytes of the MC portal
+ * @flags: flags for the new MC I/O object
+ * @new_mc_io: Area to return pointer to newly created MC I/O object
+ *
+ * Returns '0' on Success; Error code otherwise.
+ */
+int __must_check fsl_create_mc_io(struct device *dev,
+ phys_addr_t mc_portal_phys_addr,
+ uint32_t mc_portal_size,
+ uint32_t flags, struct fsl_mc_io **new_mc_io)
+{
+ struct fsl_mc_io *mc_io;
+ void __iomem *mc_portal_virt_addr;
+ struct resource *res;
+
+ mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
+ if (mc_io == NULL)
+ return -ENOMEM;
+
+ mc_io-dev = dev;
+ mc_io-flags = flags;
+ mc_io-portal_phys_addr = mc_portal_phys_addr;
+ mc_io-portal_size = mc_portal_size;
+ res = devm_request_mem_region(dev,
+ mc_portal_phys_addr,
+ mc_portal_size,
+ mc_portal);
+ if (res == NULL) {
+ dev_err(dev,
+ devm_request_mem_region failed for MC portal %#llx\n,
+ mc_portal_phys_addr);
+ return -EBUSY;
+ }
+
+ mc_portal_virt_addr = devm_ioremap_nocache(dev,
+ mc_portal_phys_addr,
+ mc_portal_size);
While I can't complain about the device itself, I will note that I think
it's a pretty bad design decision to expose actual host physical
addresses in the protocol.
I tend to agree. I'll look into creating a proposed change to the architecture
to have the MC communicate a physical offset of some kind.
Basically this means that you won't be able to pass a full MC complex
into a guest, even if you could virtualize IRQ and DMA access unless you
map it at the exact same location as the host's MC complex.
Right. But is that really an issue in practice?
Could we at least add a ranges property to the MC device description
and check whether the physical addresses we get are within that range -
if nothing else, at least as sanity check? Then maybe add some calls in
the next version that act on that range rather than actual host physical
addresses?
So you mean something like:
fsl_mc: fsl-mc@80c00 {
compatible = fsl,qoriq-mc;
#stream-id-cells = 2;
reg = 0x0008 0x0c00 0 0x40,/* MC portal base */
0x 0x0834 0 0x4; /* MC control reg */
ranges = 0x8 0x0 0x8 0x0 0x2000;
lpi-parent = its;
};
The physical addresses returned by the MC fall into a 512MB portal
region at 0x8__ in the physical address map. For now map it 1:1, but
in the
future it could become:
ranges = 0x0 0x0 0x8 0x0 0x2000;
...if I can get the hardware architecture changed.
Stuart
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 01.12.14 23:28, Stuart Yoder wrote:
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, November 27, 2014 8:30 AM
To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
linux-kernel@vger.kernel.org
Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
Bogdan-BHAMCIU1; Marginean
Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
Nir-RM30794; Schmitt Richard-B43082
Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
APIs
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
[...]
+/*
+ * Object descriptor, returned from dprc_get_obj()
+ */
+struct dprc_obj_desc {
+ /* Type of object: NULL terminated string */
+ char type[16];
I don't see where it actually gets NULL terminated - all 16 bytes come
directly from the device.
While it's probably ok to trust it, I think we'd still be safer off if
we just make this a char[17] array to always have our NULL terminating
string. That way we're guaranteed we'll never run over our memory
boundaries.
The device is supposed to guarantee that the string is null
terminated...so there will never be valid chars in the 16th
character. So, what about just forcing type[15] = '\0'?
I think that would be better than making it a char[17].
Sure, that works too.
Alex
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 01.12.14 23:53, Stuart Yoder wrote:
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, November 27, 2014 10:14 AM
To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
linux-kernel@vger.kernel.org
Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
Bogdan-BHAMCIU1; Marginean
Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
Nir-RM30794; Schmitt Richard-B43082
Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
APIs
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
[...]
+/**
+ * Creates an MC I/O object
+ *
+ * @dev: device to be associated with the MC I/O object
+ * @mc_portal_phys_addr: physical address of the MC portal to use
+ * @mc_portal_size: size in bytes of the MC portal
+ * @flags: flags for the new MC I/O object
+ * @new_mc_io: Area to return pointer to newly created MC I/O object
+ *
+ * Returns '0' on Success; Error code otherwise.
+ */
+int __must_check fsl_create_mc_io(struct device *dev,
+ phys_addr_t mc_portal_phys_addr,
+ uint32_t mc_portal_size,
+ uint32_t flags, struct fsl_mc_io **new_mc_io)
+{
+ struct fsl_mc_io *mc_io;
+ void __iomem *mc_portal_virt_addr;
+ struct resource *res;
+
+ mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
+ if (mc_io == NULL)
+ return -ENOMEM;
+
+ mc_io-dev = dev;
+ mc_io-flags = flags;
+ mc_io-portal_phys_addr = mc_portal_phys_addr;
+ mc_io-portal_size = mc_portal_size;
+ res = devm_request_mem_region(dev,
+ mc_portal_phys_addr,
+ mc_portal_size,
+ mc_portal);
+ if (res == NULL) {
+ dev_err(dev,
+ devm_request_mem_region failed for MC portal %#llx\n,
+ mc_portal_phys_addr);
+ return -EBUSY;
+ }
+
+ mc_portal_virt_addr = devm_ioremap_nocache(dev,
+ mc_portal_phys_addr,
+ mc_portal_size);
While I can't complain about the device itself, I will note that I think
it's a pretty bad design decision to expose actual host physical
addresses in the protocol.
I tend to agree. I'll look into creating a proposed change to the
architecture
to have the MC communicate a physical offset of some kind.
Basically this means that you won't be able to pass a full MC complex
into a guest, even if you could virtualize IRQ and DMA access unless you
map it at the exact same location as the host's MC complex.
Right. But is that really an issue in practice?
Well, it obviously depends on what you're trying to do. For everything
that's envisioned today I don't think it's a problem, but I like to
stick to the know as little as you have to know rule when it comes to
communication protocols.
Could we at least add a ranges property to the MC device description
and check whether the physical addresses we get are within that range -
if nothing else, at least as sanity check? Then maybe add some calls in
the next version that act on that range rather than actual host physical
addresses?
So you mean something like:
fsl_mc: fsl-mc@80c00 {
compatible = fsl,qoriq-mc;
#stream-id-cells = 2;
reg = 0x0008 0x0c00 0 0x40,/* MC portal base */
0x 0x0834 0 0x4; /* MC control reg */
ranges = 0x8 0x0 0x8 0x0 0x2000;
lpi-parent = its;
};
The physical addresses returned by the MC fall into a 512MB portal
region at 0x8__ in the physical address map. For now map it 1:1, but
in the
future it could become:
ranges = 0x0 0x0 0x8 0x0 0x2000;
...if I can get the hardware architecture changed.
Yup, I think that makes things a lot less error prone - you don't
randomly access any pointer the device tells you to access :).
Alex
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote:
> APIs to access the Management Complex (MC) hardware
> module of Freescale LS2 SoCs. This patch includes
> APIs to check the MC firmware version and to manipulate
> DPRC objects in the MC.
>
> Signed-off-by: J. German Rivera
> Signed-off-by: Stuart Yoder
[...]
> +/**
> + * Creates an MC I/O object
> + *
> + * @dev: device to be associated with the MC I/O object
> + * @mc_portal_phys_addr: physical address of the MC portal to use
> + * @mc_portal_size: size in bytes of the MC portal
> + * @flags: flags for the new MC I/O object
> + * @new_mc_io: Area to return pointer to newly created MC I/O object
> + *
> + * Returns '0' on Success; Error code otherwise.
> + */
> +int __must_check fsl_create_mc_io(struct device *dev,
> + phys_addr_t mc_portal_phys_addr,
> + uint32_t mc_portal_size,
> + uint32_t flags, struct fsl_mc_io **new_mc_io)
> +{
> + struct fsl_mc_io *mc_io;
> + void __iomem *mc_portal_virt_addr;
> + struct resource *res;
> +
> + mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
> + if (mc_io == NULL)
> + return -ENOMEM;
> +
> + mc_io->dev = dev;
> + mc_io->flags = flags;
> + mc_io->portal_phys_addr = mc_portal_phys_addr;
> + mc_io->portal_size = mc_portal_size;
> + res = devm_request_mem_region(dev,
> + mc_portal_phys_addr,
> + mc_portal_size,
> + "mc_portal");
> + if (res == NULL) {
> + dev_err(dev,
> + "devm_request_mem_region failed for MC portal %#llx\n",
> + mc_portal_phys_addr);
> + return -EBUSY;
> + }
> +
> + mc_portal_virt_addr = devm_ioremap_nocache(dev,
> +mc_portal_phys_addr,
> +mc_portal_size);
While I can't complain about the device itself, I will note that I think
it's a pretty bad design decision to expose actual host physical
addresses in the protocol.
Basically this means that you won't be able to pass a full MC complex
into a guest, even if you could virtualize IRQ and DMA access unless you
map it at the exact same location as the host's MC complex.
Could we at least add a "ranges" property to the MC device description
and check whether the physical addresses we get are within that range -
if nothing else, at least as sanity check? Then maybe add some calls in
the next version that act on that range rather than actual host physical
addresses?
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote:
> APIs to access the Management Complex (MC) hardware
> module of Freescale LS2 SoCs. This patch includes
> APIs to check the MC firmware version and to manipulate
> DPRC objects in the MC.
>
> Signed-off-by: J. German Rivera
> Signed-off-by: Stuart Yoder
[...]
> +/*
> + * Object descriptor, returned from dprc_get_obj()
> + */
> +struct dprc_obj_desc {
> + /* Type of object: NULL terminated string */
> + char type[16];
I don't see where it actually gets NULL terminated - all 16 bytes come
directly from the device.
While it's probably ok to trust it, I think we'd still be safer off if
we just make this a char[17] array to always have our NULL terminating
string. That way we're guaranteed we'll never run over our memory
boundaries.
Also sorry for the slowly trickling in comments - I'm just noting things
as I dig through the whole interface :).
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
[...]
+/*
+ * Object descriptor, returned from dprc_get_obj()
+ */
+struct dprc_obj_desc {
+ /* Type of object: NULL terminated string */
+ char type[16];
I don't see where it actually gets NULL terminated - all 16 bytes come
directly from the device.
While it's probably ok to trust it, I think we'd still be safer off if
we just make this a char[17] array to always have our NULL terminating
string. That way we're guaranteed we'll never run over our memory
boundaries.
Also sorry for the slowly trickling in comments - I'm just noting things
as I dig through the whole interface :).
Alex
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
[...]
+/**
+ * Creates an MC I/O object
+ *
+ * @dev: device to be associated with the MC I/O object
+ * @mc_portal_phys_addr: physical address of the MC portal to use
+ * @mc_portal_size: size in bytes of the MC portal
+ * @flags: flags for the new MC I/O object
+ * @new_mc_io: Area to return pointer to newly created MC I/O object
+ *
+ * Returns '0' on Success; Error code otherwise.
+ */
+int __must_check fsl_create_mc_io(struct device *dev,
+ phys_addr_t mc_portal_phys_addr,
+ uint32_t mc_portal_size,
+ uint32_t flags, struct fsl_mc_io **new_mc_io)
+{
+ struct fsl_mc_io *mc_io;
+ void __iomem *mc_portal_virt_addr;
+ struct resource *res;
+
+ mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
+ if (mc_io == NULL)
+ return -ENOMEM;
+
+ mc_io-dev = dev;
+ mc_io-flags = flags;
+ mc_io-portal_phys_addr = mc_portal_phys_addr;
+ mc_io-portal_size = mc_portal_size;
+ res = devm_request_mem_region(dev,
+ mc_portal_phys_addr,
+ mc_portal_size,
+ mc_portal);
+ if (res == NULL) {
+ dev_err(dev,
+ devm_request_mem_region failed for MC portal %#llx\n,
+ mc_portal_phys_addr);
+ return -EBUSY;
+ }
+
+ mc_portal_virt_addr = devm_ioremap_nocache(dev,
+mc_portal_phys_addr,
+mc_portal_size);
While I can't complain about the device itself, I will note that I think
it's a pretty bad design decision to expose actual host physical
addresses in the protocol.
Basically this means that you won't be able to pass a full MC complex
into a guest, even if you could virtualize IRQ and DMA access unless you
map it at the exact same location as the host's MC complex.
Could we at least add a ranges property to the MC device description
and check whether the physical addresses we get are within that range -
if nothing else, at least as sanity check? Then maybe add some calls in
the next version that act on that range rather than actual host physical
addresses?
Alex
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
> >>> +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) > >> > >> This one is definitely a misnomer. It's a command that operates on the > >> MC object, not a DPRC object. Also it doesn't fetch a random > >> "container_id", it fetches the root container id. > > > > It's not strictly the root container. It fetches the container/DPRC ID > > associated with the portal you are using. A virtual machine would use > > it to fetch it's container ID. > > So does every portal properly react to this call? Yes, they should. > Or do only container > portals react to it? Every portal is associated with some contianer-- either built into the container/DPRC object, or in a container/DPRC. > In fact, what does make the initial portal special? Nothing...it has the same semantics as any other portal. > Who reacts to > DPMNG_CMDID_foo calls? Every DPRC or only the initial root? All portals. It just means 'what container am I in?' > >> Please move it and its definition to the files that operate on the MC > >> management interface. > > > > Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID. > > > > We can request that the binary interface naming be changed, but wouldn't > > it be better to keep the functions separated by opcode type-- having > > DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands > > in a separate file? > > It really depends on what the semantics are. If this is a call that's > only valid on the MC root, then it should belong there. If it's > available on every container portal, it should be part of dprc of course. It is valid on any portal, including container portals. For that reason, I do think it may make sense for it to be a DPMNG* command. But, I would say leave the files implementing the opcode groupings alone until the MC architecture is changed. Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 26.11.14 23:33, Stuart Yoder wrote:
>
>
>> -Original Message-
>> From: Alexander Graf [mailto:ag...@suse.de]
>> Sent: Wednesday, November 26, 2014 4:16 AM
>> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
>> linux-kernel@vger.kernel.org
>> Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
>> Bogdan-BHAMCIU1; Marginean
>> Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
>> Nir-RM30794; Schmitt Richard-B43082
>> Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
>> APIs
>>
>>
>>
>> On 13.11.14 18:54, J. German Rivera wrote:
>>> APIs to access the Management Complex (MC) hardware
>>> module of Freescale LS2 SoCs. This patch includes
>>> APIs to check the MC firmware version and to manipulate
>>> DPRC objects in the MC.
>>>
>>> Signed-off-by: J. German Rivera
>>> Signed-off-by: Stuart Yoder
>>
>> [...]
>>
>>> diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
>>> new file mode 100644
>>> index 000..40ae552
>>> --- /dev/null
>>> +++ b/drivers/bus/fsl-mc/dprc.c
>>> @@ -0,0 +1,933 @@
>>> +/* Copyright 2013-2014 Freescale Semiconductor Inc.
>>> +*
>>> +* Redistribution and use in source and binary forms, with or without
>>> +* modification, are permitted provided that the following conditions are
>>> met:
>>> +* * Redistributions of source code must retain the above copyright
>>> +* notice, this list of conditions and the following disclaimer.
>>> +* * Redistributions in binary form must reproduce the above copyright
>>> +* notice, this list of conditions and the following disclaimer in the
>>> +* documentation and/or other materials provided with the distribution.
>>> +* * Neither the name of the above-listed copyright holders nor the
>>> +* names of any contributors may be used to endorse or promote products
>>> +* derived from this software without specific prior written permission.
>>> +*
>>> +*
>>> +* ALTERNATIVELY, this software may be distributed under the terms of the
>>> +* GNU General Public License ("GPL") as published by the Free Software
>>> +* Foundation, either version 2 of that License or (at your option) any
>>> +* later version.
>>> +*
>>> +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
>>> IS"
>>> +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>>> +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>>> PURPOSE
>>> +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS
>>> BE
>>> +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>>> +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>>> +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>>> +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>>> +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>>> +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>>> THE
>>> +* POSSIBILITY OF SUCH DAMAGE.
>>> +*/
>>> +#include
>>> +#include
>>> +#include
>>> +#include "dprc-cmd.h"
>>> +
>>> +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id)
>>
>> This one is definitely a misnomer. It's a command that operates on the
>> MC object, not a DPRC object. Also it doesn't fetch a random
>> "container_id", it fetches the root container id.
>
> It's not strictly the root container. It fetches the container/DPRC ID
> associated with the portal you are using. A virtual machine would use
> it to fetch it's container ID.
So does every portal properly react to this call? Or do only container
portals react to it?
In fact, what does make the initial portal special? Who reacts to
DPMNG_CMDID_foo calls? Every DPRC or only the initial root?
>> Please move it and its definition to the files that operate on the MC
>> management interface.
>
> Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID.
>
> We can request that the binary interface naming be changed, but wouldn't
> it be better to keep the functions separated by opcode type-- having
> DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands
> in a separate file?
It really depends on what the semantics are. If this is a call that's
only valid on the MC root, then it should belong there. If it's
available on every container portal, it should be part of dprc of course.
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Wednesday, November 26, 2014 4:16 AM
> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
> linux-kernel@vger.kernel.org
> Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc
> Bogdan-BHAMCIU1; Marginean
> Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez
> Nir-RM30794; Schmitt Richard-B43082
> Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex
> APIs
>
>
>
> On 13.11.14 18:54, J. German Rivera wrote:
> > APIs to access the Management Complex (MC) hardware
> > module of Freescale LS2 SoCs. This patch includes
> > APIs to check the MC firmware version and to manipulate
> > DPRC objects in the MC.
> >
> > Signed-off-by: J. German Rivera
> > Signed-off-by: Stuart Yoder
>
> [...]
>
> > diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
> > new file mode 100644
> > index 000..40ae552
> > --- /dev/null
> > +++ b/drivers/bus/fsl-mc/dprc.c
> > @@ -0,0 +1,933 @@
> > +/* Copyright 2013-2014 Freescale Semiconductor Inc.
> > +*
> > +* Redistribution and use in source and binary forms, with or without
> > +* modification, are permitted provided that the following conditions are
> > met:
> > +* * Redistributions of source code must retain the above copyright
> > +* notice, this list of conditions and the following disclaimer.
> > +* * Redistributions in binary form must reproduce the above copyright
> > +* notice, this list of conditions and the following disclaimer in the
> > +* documentation and/or other materials provided with the distribution.
> > +* * Neither the name of the above-listed copyright holders nor the
> > +* names of any contributors may be used to endorse or promote products
> > +* derived from this software without specific prior written permission.
> > +*
> > +*
> > +* ALTERNATIVELY, this software may be distributed under the terms of the
> > +* GNU General Public License ("GPL") as published by the Free Software
> > +* Foundation, either version 2 of that License or (at your option) any
> > +* later version.
> > +*
> > +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> > IS"
> > +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> > PURPOSE
> > +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS
> > BE
> > +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> > +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> > +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> > +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> > +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
> > THE
> > +* POSSIBILITY OF SUCH DAMAGE.
> > +*/
> > +#include
> > +#include
> > +#include
> > +#include "dprc-cmd.h"
> > +
> > +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id)
>
> This one is definitely a misnomer. It's a command that operates on the
> MC object, not a DPRC object. Also it doesn't fetch a random
> "container_id", it fetches the root container id.
It's not strictly the root container. It fetches the container/DPRC ID
associated with the portal you are using. A virtual machine would use
it to fetch it's container ID.
> Please move it and its definition to the files that operate on the MC
> management interface.
Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID.
We can request that the binary interface naming be changed, but wouldn't
it be better to keep the functions separated by opcode type-- having
DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands
in a separate file?
We have already submitted a request to move this opcode to the DPMNG
group.
Thanks,
Stuart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 11/26/2014 04:15 AM, Alexander Graf wrote: On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera Signed-off-by: Stuart Yoder [...] [] + +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random "container_id", it fetches the root container id. Please move it and its definition to the files that operate on the MC management interface. Ok, I will make this change in respin v5. Thanks, German Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote:
> APIs to access the Management Complex (MC) hardware
> module of Freescale LS2 SoCs. This patch includes
> APIs to check the MC firmware version and to manipulate
> DPRC objects in the MC.
>
> Signed-off-by: J. German Rivera
> Signed-off-by: Stuart Yoder
[...]
> diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
> new file mode 100644
> index 000..40ae552
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dprc.c
> @@ -0,0 +1,933 @@
> +/* Copyright 2013-2014 Freescale Semiconductor Inc.
> +*
> +* Redistribution and use in source and binary forms, with or without
> +* modification, are permitted provided that the following conditions are met:
> +* * Redistributions of source code must retain the above copyright
> +* notice, this list of conditions and the following disclaimer.
> +* * Redistributions in binary form must reproduce the above copyright
> +* notice, this list of conditions and the following disclaimer in the
> +* documentation and/or other materials provided with the distribution.
> +* * Neither the name of the above-listed copyright holders nor the
> +* names of any contributors may be used to endorse or promote products
> +* derived from this software without specific prior written permission.
> +*
> +*
> +* ALTERNATIVELY, this software may be distributed under the terms of the
> +* GNU General Public License ("GPL") as published by the Free Software
> +* Foundation, either version 2 of that License or (at your option) any
> +* later version.
> +*
> +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +* POSSIBILITY OF SUCH DAMAGE.
> +*/
> +#include
> +#include
> +#include
> +#include "dprc-cmd.h"
> +
> +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id)
This one is definitely a misnomer. It's a command that operates on the
MC object, not a DPRC object. Also it doesn't fetch a random
"container_id", it fetches the root container id.
Please move it and its definition to the files that operate on the MC
management interface.
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com [...] diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c new file mode 100644 index 000..40ae552 --- /dev/null +++ b/drivers/bus/fsl-mc/dprc.c @@ -0,0 +1,933 @@ +/* Copyright 2013-2014 Freescale Semiconductor Inc. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are met: +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in the +* documentation and/or other materials provided with the distribution. +* * Neither the name of the above-listed copyright holders nor the +* names of any contributors may be used to endorse or promote products +* derived from this software without specific prior written permission. +* +* +* ALTERNATIVELY, this software may be distributed under the terms of the +* GNU General Public License (GPL) as published by the Free Software +* Foundation, either version 2 of that License or (at your option) any +* later version. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS IS +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE. +*/ +#include linux/fsl/mc-sys.h +#include linux/fsl/mc-cmd.h +#include linux/fsl/dprc.h +#include dprc-cmd.h + +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random container_id, it fetches the root container id. Please move it and its definition to the files that operate on the MC management interface. Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 11/26/2014 04:15 AM, Alexander Graf wrote: On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com [...] [] + +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random container_id, it fetches the root container id. Please move it and its definition to the files that operate on the MC management interface. Ok, I will make this change in respin v5. Thanks, German Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, November 26, 2014 4:16 AM To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard-B43082 Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com [...] diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c new file mode 100644 index 000..40ae552 --- /dev/null +++ b/drivers/bus/fsl-mc/dprc.c @@ -0,0 +1,933 @@ +/* Copyright 2013-2014 Freescale Semiconductor Inc. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are met: +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in the +* documentation and/or other materials provided with the distribution. +* * Neither the name of the above-listed copyright holders nor the +* names of any contributors may be used to endorse or promote products +* derived from this software without specific prior written permission. +* +* +* ALTERNATIVELY, this software may be distributed under the terms of the +* GNU General Public License (GPL) as published by the Free Software +* Foundation, either version 2 of that License or (at your option) any +* later version. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS IS +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE. +*/ +#include linux/fsl/mc-sys.h +#include linux/fsl/mc-cmd.h +#include linux/fsl/dprc.h +#include dprc-cmd.h + +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random container_id, it fetches the root container id. It's not strictly the root container. It fetches the container/DPRC ID associated with the portal you are using. A virtual machine would use it to fetch it's container ID. Please move it and its definition to the files that operate on the MC management interface. Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID. We can request that the binary interface naming be changed, but wouldn't it be better to keep the functions separated by opcode type-- having DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands in a separate file? We have already submitted a request to move this opcode to the DPMNG group. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 26.11.14 23:33, Stuart Yoder wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, November 26, 2014 4:16 AM To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard-B43082 Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com [...] diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c new file mode 100644 index 000..40ae552 --- /dev/null +++ b/drivers/bus/fsl-mc/dprc.c @@ -0,0 +1,933 @@ +/* Copyright 2013-2014 Freescale Semiconductor Inc. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are met: +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in the +* documentation and/or other materials provided with the distribution. +* * Neither the name of the above-listed copyright holders nor the +* names of any contributors may be used to endorse or promote products +* derived from this software without specific prior written permission. +* +* +* ALTERNATIVELY, this software may be distributed under the terms of the +* GNU General Public License (GPL) as published by the Free Software +* Foundation, either version 2 of that License or (at your option) any +* later version. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS IS +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE. +*/ +#include linux/fsl/mc-sys.h +#include linux/fsl/mc-cmd.h +#include linux/fsl/dprc.h +#include dprc-cmd.h + +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random container_id, it fetches the root container id. It's not strictly the root container. It fetches the container/DPRC ID associated with the portal you are using. A virtual machine would use it to fetch it's container ID. So does every portal properly react to this call? Or do only container portals react to it? In fact, what does make the initial portal special? Who reacts to DPMNG_CMDID_foo calls? Every DPRC or only the initial root? Please move it and its definition to the files that operate on the MC management interface. Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID. We can request that the binary interface naming be changed, but wouldn't it be better to keep the functions separated by opcode type-- having DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands in a separate file? It really depends on what the semantics are. If this is a call that's only valid on the MC root, then it should belong there. If it's available on every container portal, it should be part of dprc of course. Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
+int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random container_id, it fetches the root container id. It's not strictly the root container. It fetches the container/DPRC ID associated with the portal you are using. A virtual machine would use it to fetch it's container ID. So does every portal properly react to this call? Yes, they should. Or do only container portals react to it? Every portal is associated with some contianer-- either built into the container/DPRC object, or in a container/DPRC. In fact, what does make the initial portal special? Nothing...it has the same semantics as any other portal. Who reacts to DPMNG_CMDID_foo calls? Every DPRC or only the initial root? All portals. It just means 'what container am I in?' Please move it and its definition to the files that operate on the MC management interface. Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID. We can request that the binary interface naming be changed, but wouldn't it be better to keep the functions separated by opcode type-- having DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands in a separate file? It really depends on what the semantics are. If this is a call that's only valid on the MC root, then it should belong there. If it's available on every container portal, it should be part of dprc of course. It is valid on any portal, including container portals. For that reason, I do think it may make sense for it to be a DPMNG* command. But, I would say leave the files implementing the opcode groupings alone until the MC architecture is changed. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 11/25/2014 08:06 AM, Alexander Graf wrote:
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera
Signed-off-by: Stuart Yoder
---
Changes in v4:
- Addressed comments from Alex Graf:
* Added file description for files that did not have one
* Removed Marshalling/unmarshalling macros invoked in MC
command wrapper functions, and instead do the marshalling/
unmarshalling directly in these functions.
* Added type cast u32 in status range-check in mc_status_to_error()
* Moved mc_write_command() and mc_read_response() inline
function to mc_sys.c as they are only used in that file
* Renamed u64_enc() as mc_enc() and u64_dec() as mc_dec()
- Upgraded MC flibs for MC firmware 0.5
Changes in v3:
- Addressed comment from Greg Kroah-Hartman:
* Removed doxygen comments
- Addressed comment from Scott Wood:
* Replaced udelay() call with usleep_range() call in polling loop
- Addressed comments from Kim Phillips:
* Fixed license text in all files
* Renamed files:
drivers/bus/fsl-mc/fsl_dpmng_cmd.h -> drivers/bus/fsl-mc/dpmng-cmd.h
drivers/bus/fsl-mc/fsl_dprc_cmd.h -> drivers/bus/fsl-mc/dprc-cmd.h
drivers/bus/fsl-mc/fsl_mc_sys.c -> drivers/bus/fsl-mc/mc-sys.c
include/linux/fsl_dpmng.h -> include/linux/fsl/dpmng.h
include/linux/fsl_dprc.h -> include/linux/fsl/dprc.h
include/linux/fsl_mc_cmd.h -> include/linux/fsl/mc-cmd.h
include/linux/fsl_mc_sys.h -> include/linux/fsl/mc-sys.h
* Changed dpmng_load_iop() to take the DMA address directly,
instead of the image virtual address.
* Removed if and WARN_ON that checks for NULL fsl_destroy_mc_io()
* Removed locking from mc_send_command(). Now the caller of MC flib
APIs is responsible for protecting concurrent accesses to the same
MC portal.
Changes in v2:
- Addressed comment from Joe Perches:
* Refactored logic to actively fail on err and proceed at
the same indent level on success, for all functions in dprc.c
and dpmng.c.
- Addressed comments from Kim Phillips:
* Fixed warning in mc_send_command
* Changed serialization logic in mc_send_command() to only use
spinlock_irqsave() when both threads and interrupt handlers
concurrently access the same portal.
* Changed switch to lookup table in mc_status_to_error()
* Removed macros iowrite64(), ioread64(), ENOTSUP from fsl_mc_sys.h
* Removed #ifdef side for FSL_MC_FIRMWARE in fsl_mc_cmd.h
* Changed non-devm_ API calls to devm_ API calls and refactored
fsl_create_mc_io()/fsl_destroy_mc_io() to simplify cleanup logic.
- Addressed comments from Scott Wood:
* Return -ENXIO instead of -EFAULT when ioremap_nocache() fails
- Addressed comments from Alex Graf:
* Added MAINTAINERS file entries for new files
* Added dev param to fsl_create_mc_io(), to enable the use
of devm_ APIs in this function and fsl_destroy_mc_io().
* Changed the value of the timeout for MC command completion
to be a function of HZ instead of a hard-coded jiffies value.
MAINTAINERS| 8 +
drivers/bus/fsl-mc/dpmng-cmd.h | 50 +++
drivers/bus/fsl-mc/dpmng.c | 126 ++
drivers/bus/fsl-mc/dprc-cmd.h | 85
drivers/bus/fsl-mc/dprc.c | 933 +
drivers/bus/fsl-mc/mc-sys.c| 284 +
include/linux/fsl/dpmng.h | 151 +++
include/linux/fsl/dprc.h | 868 ++
include/linux/fsl/mc-cmd.h | 109 +
include/linux/fsl/mc-sys.h | 70
10 files changed, 2684 insertions(+)
create mode 100644 drivers/bus/fsl-mc/dpmng-cmd.h
create mode 100644 drivers/bus/fsl-mc/dpmng.c
create mode 100644 drivers/bus/fsl-mc/dprc-cmd.h
create mode 100644 drivers/bus/fsl-mc/dprc.c
create mode 100644 drivers/bus/fsl-mc/mc-sys.c
create mode 100644 include/linux/fsl/dpmng.h
create mode 100644 include/linux/fsl/dprc.h
create mode 100644 include/linux/fsl/mc-cmd.h
create mode 100644 include/linux/fsl/mc-sys.h
[...]
+/**
+ * Sends an command to the MC device using the given MC I/O object
+ *
+ * @mc_io: MC I/O object to be used
+ * @cmd: command to be sent
+ *
+ * Returns '0' on Success; Error code otherwise.
+ *
+ * NOTE: This function cannot be invoked from from atomic contexts.
+ */
+int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
+{
+ enum mc_cmd_status status;
+ unsigned long irqsave_flags = 0;
drivers/bus/fsl-mc/mc-sys.c: In function ‘mc_send_command’:
drivers/bus/fsl-mc/mc-sys.c:235:16: warning: unused variable
‘irqsave_flags’ [-Wunused-variable]
unsigned long irqsave_flags = 0;
I'll fix this in the next respin.
Thanks,
German
^
Alex
--
To unsubscribe from this
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote:
> APIs to access the Management Complex (MC) hardware
> module of Freescale LS2 SoCs. This patch includes
> APIs to check the MC firmware version and to manipulate
> DPRC objects in the MC.
>
> Signed-off-by: J. German Rivera
> Signed-off-by: Stuart Yoder
> ---
> Changes in v4:
> - Addressed comments from Alex Graf:
> * Added file description for files that did not have one
> * Removed Marshalling/unmarshalling macros invoked in MC
> command wrapper functions, and instead do the marshalling/
> unmarshalling directly in these functions.
> * Added type cast u32 in status range-check in mc_status_to_error()
> * Moved mc_write_command() and mc_read_response() inline
> function to mc_sys.c as they are only used in that file
> * Renamed u64_enc() as mc_enc() and u64_dec() as mc_dec()
> - Upgraded MC flibs for MC firmware 0.5
>
> Changes in v3:
> - Addressed comment from Greg Kroah-Hartman:
> * Removed doxygen comments
>
> - Addressed comment from Scott Wood:
> * Replaced udelay() call with usleep_range() call in polling loop
> - Addressed comments from Kim Phillips:
> * Fixed license text in all files
> * Renamed files:
> drivers/bus/fsl-mc/fsl_dpmng_cmd.h -> drivers/bus/fsl-mc/dpmng-cmd.h
> drivers/bus/fsl-mc/fsl_dprc_cmd.h -> drivers/bus/fsl-mc/dprc-cmd.h
> drivers/bus/fsl-mc/fsl_mc_sys.c -> drivers/bus/fsl-mc/mc-sys.c
> include/linux/fsl_dpmng.h -> include/linux/fsl/dpmng.h
> include/linux/fsl_dprc.h -> include/linux/fsl/dprc.h
> include/linux/fsl_mc_cmd.h -> include/linux/fsl/mc-cmd.h
> include/linux/fsl_mc_sys.h -> include/linux/fsl/mc-sys.h
> * Changed dpmng_load_iop() to take the DMA address directly,
> instead of the image virtual address.
> * Removed if and WARN_ON that checks for NULL fsl_destroy_mc_io()
> * Removed locking from mc_send_command(). Now the caller of MC flib
> APIs is responsible for protecting concurrent accesses to the same
> MC portal.
>
> Changes in v2:
> - Addressed comment from Joe Perches:
> * Refactored logic to actively fail on err and proceed at
> the same indent level on success, for all functions in dprc.c
> and dpmng.c.
>
> - Addressed comments from Kim Phillips:
> * Fixed warning in mc_send_command
> * Changed serialization logic in mc_send_command() to only use
> spinlock_irqsave() when both threads and interrupt handlers
> concurrently access the same portal.
> * Changed switch to lookup table in mc_status_to_error()
> * Removed macros iowrite64(), ioread64(), ENOTSUP from fsl_mc_sys.h
> * Removed #ifdef side for FSL_MC_FIRMWARE in fsl_mc_cmd.h
> * Changed non-devm_ API calls to devm_ API calls and refactored
> fsl_create_mc_io()/fsl_destroy_mc_io() to simplify cleanup logic.
>
> - Addressed comments from Scott Wood:
> * Return -ENXIO instead of -EFAULT when ioremap_nocache() fails
>
> - Addressed comments from Alex Graf:
> * Added MAINTAINERS file entries for new files
> * Added dev param to fsl_create_mc_io(), to enable the use
> of devm_ APIs in this function and fsl_destroy_mc_io().
> * Changed the value of the timeout for MC command completion
> to be a function of HZ instead of a hard-coded jiffies value.
>
> MAINTAINERS| 8 +
> drivers/bus/fsl-mc/dpmng-cmd.h | 50 +++
> drivers/bus/fsl-mc/dpmng.c | 126 ++
> drivers/bus/fsl-mc/dprc-cmd.h | 85
> drivers/bus/fsl-mc/dprc.c | 933
> +
> drivers/bus/fsl-mc/mc-sys.c| 284 +
> include/linux/fsl/dpmng.h | 151 +++
> include/linux/fsl/dprc.h | 868 ++
> include/linux/fsl/mc-cmd.h | 109 +
> include/linux/fsl/mc-sys.h | 70
> 10 files changed, 2684 insertions(+)
> create mode 100644 drivers/bus/fsl-mc/dpmng-cmd.h
> create mode 100644 drivers/bus/fsl-mc/dpmng.c
> create mode 100644 drivers/bus/fsl-mc/dprc-cmd.h
> create mode 100644 drivers/bus/fsl-mc/dprc.c
> create mode 100644 drivers/bus/fsl-mc/mc-sys.c
> create mode 100644 include/linux/fsl/dpmng.h
> create mode 100644 include/linux/fsl/dprc.h
> create mode 100644 include/linux/fsl/mc-cmd.h
> create mode 100644 include/linux/fsl/mc-sys.h
>
[...]
> +/**
> + * Sends an command to the MC device using the given MC I/O object
> + *
> + * @mc_io: MC I/O object to be used
> + * @cmd: command to be sent
> + *
> + * Returns '0' on Success; Error code otherwise.
> + *
> + * NOTE: This function cannot be invoked from from atomic contexts.
> + */
> +int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
> +{
> + enum mc_cmd_status status;
> + unsigned long irqsave_flags = 0;
drivers/bus/fsl-mc/mc-sys.c: In function ‘mc_send_command’:
drivers/bus/fsl-mc/mc-sys.c:235:16: warning: unused variable
‘irqsave_flags’ [-Wunused-variable]
unsigned long irqsave_flags = 0;
^
Alex
--
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
---
Changes in v4:
- Addressed comments from Alex Graf:
* Added file description for files that did not have one
* Removed Marshalling/unmarshalling macros invoked in MC
command wrapper functions, and instead do the marshalling/
unmarshalling directly in these functions.
* Added type cast u32 in status range-check in mc_status_to_error()
* Moved mc_write_command() and mc_read_response() inline
function to mc_sys.c as they are only used in that file
* Renamed u64_enc() as mc_enc() and u64_dec() as mc_dec()
- Upgraded MC flibs for MC firmware 0.5
Changes in v3:
- Addressed comment from Greg Kroah-Hartman:
* Removed doxygen comments
- Addressed comment from Scott Wood:
* Replaced udelay() call with usleep_range() call in polling loop
- Addressed comments from Kim Phillips:
* Fixed license text in all files
* Renamed files:
drivers/bus/fsl-mc/fsl_dpmng_cmd.h - drivers/bus/fsl-mc/dpmng-cmd.h
drivers/bus/fsl-mc/fsl_dprc_cmd.h - drivers/bus/fsl-mc/dprc-cmd.h
drivers/bus/fsl-mc/fsl_mc_sys.c - drivers/bus/fsl-mc/mc-sys.c
include/linux/fsl_dpmng.h - include/linux/fsl/dpmng.h
include/linux/fsl_dprc.h - include/linux/fsl/dprc.h
include/linux/fsl_mc_cmd.h - include/linux/fsl/mc-cmd.h
include/linux/fsl_mc_sys.h - include/linux/fsl/mc-sys.h
* Changed dpmng_load_iop() to take the DMA address directly,
instead of the image virtual address.
* Removed if and WARN_ON that checks for NULL fsl_destroy_mc_io()
* Removed locking from mc_send_command(). Now the caller of MC flib
APIs is responsible for protecting concurrent accesses to the same
MC portal.
Changes in v2:
- Addressed comment from Joe Perches:
* Refactored logic to actively fail on err and proceed at
the same indent level on success, for all functions in dprc.c
and dpmng.c.
- Addressed comments from Kim Phillips:
* Fixed warning in mc_send_command
* Changed serialization logic in mc_send_command() to only use
spinlock_irqsave() when both threads and interrupt handlers
concurrently access the same portal.
* Changed switch to lookup table in mc_status_to_error()
* Removed macros iowrite64(), ioread64(), ENOTSUP from fsl_mc_sys.h
* Removed #ifdef side for FSL_MC_FIRMWARE in fsl_mc_cmd.h
* Changed non-devm_ API calls to devm_ API calls and refactored
fsl_create_mc_io()/fsl_destroy_mc_io() to simplify cleanup logic.
- Addressed comments from Scott Wood:
* Return -ENXIO instead of -EFAULT when ioremap_nocache() fails
- Addressed comments from Alex Graf:
* Added MAINTAINERS file entries for new files
* Added dev param to fsl_create_mc_io(), to enable the use
of devm_ APIs in this function and fsl_destroy_mc_io().
* Changed the value of the timeout for MC command completion
to be a function of HZ instead of a hard-coded jiffies value.
MAINTAINERS| 8 +
drivers/bus/fsl-mc/dpmng-cmd.h | 50 +++
drivers/bus/fsl-mc/dpmng.c | 126 ++
drivers/bus/fsl-mc/dprc-cmd.h | 85
drivers/bus/fsl-mc/dprc.c | 933
+
drivers/bus/fsl-mc/mc-sys.c| 284 +
include/linux/fsl/dpmng.h | 151 +++
include/linux/fsl/dprc.h | 868 ++
include/linux/fsl/mc-cmd.h | 109 +
include/linux/fsl/mc-sys.h | 70
10 files changed, 2684 insertions(+)
create mode 100644 drivers/bus/fsl-mc/dpmng-cmd.h
create mode 100644 drivers/bus/fsl-mc/dpmng.c
create mode 100644 drivers/bus/fsl-mc/dprc-cmd.h
create mode 100644 drivers/bus/fsl-mc/dprc.c
create mode 100644 drivers/bus/fsl-mc/mc-sys.c
create mode 100644 include/linux/fsl/dpmng.h
create mode 100644 include/linux/fsl/dprc.h
create mode 100644 include/linux/fsl/mc-cmd.h
create mode 100644 include/linux/fsl/mc-sys.h
[...]
+/**
+ * Sends an command to the MC device using the given MC I/O object
+ *
+ * @mc_io: MC I/O object to be used
+ * @cmd: command to be sent
+ *
+ * Returns '0' on Success; Error code otherwise.
+ *
+ * NOTE: This function cannot be invoked from from atomic contexts.
+ */
+int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
+{
+ enum mc_cmd_status status;
+ unsigned long irqsave_flags = 0;
drivers/bus/fsl-mc/mc-sys.c: In function ‘mc_send_command’:
drivers/bus/fsl-mc/mc-sys.c:235:16: warning: unused variable
‘irqsave_flags’ [-Wunused-variable]
unsigned long irqsave_flags = 0;
^
Alex
--
To unsubscribe from this list: send the line unsubscribe
Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
On 11/25/2014 08:06 AM, Alexander Graf wrote:
On 13.11.14 18:54, J. German Rivera wrote:
APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.
Signed-off-by: J. German Rivera german.riv...@freescale.com
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
---
Changes in v4:
- Addressed comments from Alex Graf:
* Added file description for files that did not have one
* Removed Marshalling/unmarshalling macros invoked in MC
command wrapper functions, and instead do the marshalling/
unmarshalling directly in these functions.
* Added type cast u32 in status range-check in mc_status_to_error()
* Moved mc_write_command() and mc_read_response() inline
function to mc_sys.c as they are only used in that file
* Renamed u64_enc() as mc_enc() and u64_dec() as mc_dec()
- Upgraded MC flibs for MC firmware 0.5
Changes in v3:
- Addressed comment from Greg Kroah-Hartman:
* Removed doxygen comments
- Addressed comment from Scott Wood:
* Replaced udelay() call with usleep_range() call in polling loop
- Addressed comments from Kim Phillips:
* Fixed license text in all files
* Renamed files:
drivers/bus/fsl-mc/fsl_dpmng_cmd.h - drivers/bus/fsl-mc/dpmng-cmd.h
drivers/bus/fsl-mc/fsl_dprc_cmd.h - drivers/bus/fsl-mc/dprc-cmd.h
drivers/bus/fsl-mc/fsl_mc_sys.c - drivers/bus/fsl-mc/mc-sys.c
include/linux/fsl_dpmng.h - include/linux/fsl/dpmng.h
include/linux/fsl_dprc.h - include/linux/fsl/dprc.h
include/linux/fsl_mc_cmd.h - include/linux/fsl/mc-cmd.h
include/linux/fsl_mc_sys.h - include/linux/fsl/mc-sys.h
* Changed dpmng_load_iop() to take the DMA address directly,
instead of the image virtual address.
* Removed if and WARN_ON that checks for NULL fsl_destroy_mc_io()
* Removed locking from mc_send_command(). Now the caller of MC flib
APIs is responsible for protecting concurrent accesses to the same
MC portal.
Changes in v2:
- Addressed comment from Joe Perches:
* Refactored logic to actively fail on err and proceed at
the same indent level on success, for all functions in dprc.c
and dpmng.c.
- Addressed comments from Kim Phillips:
* Fixed warning in mc_send_command
* Changed serialization logic in mc_send_command() to only use
spinlock_irqsave() when both threads and interrupt handlers
concurrently access the same portal.
* Changed switch to lookup table in mc_status_to_error()
* Removed macros iowrite64(), ioread64(), ENOTSUP from fsl_mc_sys.h
* Removed #ifdef side for FSL_MC_FIRMWARE in fsl_mc_cmd.h
* Changed non-devm_ API calls to devm_ API calls and refactored
fsl_create_mc_io()/fsl_destroy_mc_io() to simplify cleanup logic.
- Addressed comments from Scott Wood:
* Return -ENXIO instead of -EFAULT when ioremap_nocache() fails
- Addressed comments from Alex Graf:
* Added MAINTAINERS file entries for new files
* Added dev param to fsl_create_mc_io(), to enable the use
of devm_ APIs in this function and fsl_destroy_mc_io().
* Changed the value of the timeout for MC command completion
to be a function of HZ instead of a hard-coded jiffies value.
MAINTAINERS| 8 +
drivers/bus/fsl-mc/dpmng-cmd.h | 50 +++
drivers/bus/fsl-mc/dpmng.c | 126 ++
drivers/bus/fsl-mc/dprc-cmd.h | 85
drivers/bus/fsl-mc/dprc.c | 933 +
drivers/bus/fsl-mc/mc-sys.c| 284 +
include/linux/fsl/dpmng.h | 151 +++
include/linux/fsl/dprc.h | 868 ++
include/linux/fsl/mc-cmd.h | 109 +
include/linux/fsl/mc-sys.h | 70
10 files changed, 2684 insertions(+)
create mode 100644 drivers/bus/fsl-mc/dpmng-cmd.h
create mode 100644 drivers/bus/fsl-mc/dpmng.c
create mode 100644 drivers/bus/fsl-mc/dprc-cmd.h
create mode 100644 drivers/bus/fsl-mc/dprc.c
create mode 100644 drivers/bus/fsl-mc/mc-sys.c
create mode 100644 include/linux/fsl/dpmng.h
create mode 100644 include/linux/fsl/dprc.h
create mode 100644 include/linux/fsl/mc-cmd.h
create mode 100644 include/linux/fsl/mc-sys.h
[...]
+/**
+ * Sends an command to the MC device using the given MC I/O object
+ *
+ * @mc_io: MC I/O object to be used
+ * @cmd: command to be sent
+ *
+ * Returns '0' on Success; Error code otherwise.
+ *
+ * NOTE: This function cannot be invoked from from atomic contexts.
+ */
+int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
+{
+ enum mc_cmd_status status;
+ unsigned long irqsave_flags = 0;
drivers/bus/fsl-mc/mc-sys.c: In function ‘mc_send_command’:
drivers/bus/fsl-mc/mc-sys.c:235:16: warning: unused variable
‘irqsave_flags’ [-Wunused-variable]
unsigned long irqsave_flags = 0;
I'll fix this in the next respin.
Thanks,
German
