* [PATCH v6 4/5] ARM: DTS: da850: Add cfgchip syscon node
From: David Lechner @ 2016-10-28 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <m2fungcses.fsf@baylibre.com>
On 10/28/2016 12:08 PM, Kevin Hilman wrote:
> Sekhar Nori <nsekhar@ti.com> writes:
>
>> On Wednesday 26 October 2016 09:38 PM, David Lechner wrote:
>>> On 10/25/2016 10:06 PM, David Lechner wrote:
>>>> Add a syscon node for the SoC CFGCHIPn registers. This is needed for
>>>> the new usb phy driver.
>>>>
>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>> ---
>>>> arch/arm/boot/dts/da850.dtsi | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index f79e1b9..6bbf20d 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -188,6 +188,10 @@
>>>> };
>>>>
>>>> };
>>>> + cfgchip: cfgchip at 1417c {
>>>
>>> I wonder if there is a more generic name instead of cfgchip at . Is there a
>>> preferred generic name for syscon nodes?
>>
>> I did not find anything in ePAPR, but chip-controller might be more
>> appropriate.
>>
>>>
>>>> + compatible = "ti,da830-cfgchip", "syscon";
>>
>> Looks like we need "simple-mfd" too in the compatible list?
>>
>> I think we can also fold patch 5/5 into this patch and add the cfgchip
>> along with USB phy child node included.
>>
>> If you respin the patch, I can drop 4/5 and 5/5 that I have queued and
>> included the updated patch instead.
>
> Sekhar, what's your opinion of having this syscon just for CFGCHIP* vs
> a single syscon for the whole SYSCFG0 region.
>
> The drivers/bus driver from Bartosz is also using SYSCFG0 registers, and
> proposing a sysconf ro this region, but it will need to exclude the
> CFGCHIPn registers if we also have this syscon.
What about the pinmux registers, which are already being used separately
too?
>
> I tend to think we should just have one for the whole SYSCFG0 which
> this series could use.
>
> Unfortunately, the PHY driver is already merged and it references the
> syscon by compatible. The PHY driver should probably be fixed to find
> its syscon by phandle, and then maybe we could move to a single syscon
> for SYSCFG0?
I agree that this should be change, but I was thinking we should use
syscon_node_to_regmap(np->parent) since the phy node should be a child
of the syscon node.
>
> Let us know your preference, I don't have a very strong feeling either
> way, but since we're already part way down the path of the CFGCHIP
> syscon, we should keep it and later migrate it to one for all of
> SYSCFG0.
>
> Kevin
>
^ permalink raw reply
* pinctrl: mediatek: build failure if CONFIG_IRQ_DOMAIN is not set
From: Paul Bolle @ 2016-10-28 17:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
0) A rather spartan build, on x86_64, which did include
drivers/pinctrl/mediatek/pinctrl-mtk-common.o failed like this:
drivers/pinctrl/mediatek/pinctrl-mtk-common.c: In function ?mtk_gpio_to_irq?:
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:838:8: error: implicit declaration of function ?irq_find_mapping? [-Werror=implicit-function-declaration]
irq = irq_find_mapping(pctl->domain, pin->eint.eintnum);
^~~~~~~~~~~~~~~~
drivers/pinctrl/mediatek/pinctrl-mtk-common.c: In function ?mtk_pctrl_init?:
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1474:17: error: implicit declaration of function ?irq_domain_add_linear? [-Werror=implicit-function-declaration]
pctl->domain = irq_domain_add_linear(np,
^~~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1475:27: error: ?irq_domain_simple_ops? undeclared (first use in this function)
pctl->devdata->ap_num, &irq_domain_simple_ops, NULL);
^~~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1475:27: note: each undeclared identifier is reported only once for each function it appears in
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1484:14: error: implicit declaration of function ?irq_create_mapping? [-Werror=implicit-function-declaration]
int virq = irq_create_mapping(pctl->domain, i);
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [drivers/pinctrl/mediatek/pinctrl-mtk-common.o] Error 1
make[2]: *** [drivers/pinctrl/mediatek] Error 2
make[1]: *** [drivers/pinctrl] Error 2
make: *** [drivers] Error 2
1) That build had CONFIG_COMPILE_TEST set (obviously) but
CONFIG_IRQ_DOMAIN not set.
2) This quick hack fixes that for me:
diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig
index 419ea4d5964d..066087156dcc 100644
--- a/drivers/pinctrl/mediatek/Kconfig
+++ b/drivers/pinctrl/mediatek/Kconfig
@@ -7,6 +7,7 @@ config PINCTRL_MTK
select GENERIC_PINCONF
select GPIOLIB
select OF_GPIO
+ select IRQ_DOMAIN
# For ARMv7 SoCs
config PINCTRL_MT2701
3) Would you like me to submit a proper (but lightly tested) patch or
do you prefer to fix this yourself?
Thanks,
Paul Bolle
^ permalink raw reply related
* [RFC PATCH 0/6] media: davinci: VPIF: add DT support
From: Kevin Hilman @ 2016-10-28 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025235536.7342-1-khilman@baylibre.com>
Kevin Hilman <khilman@baylibre.com> writes:
> This series attempts to add DT support to the davinci VPIF capture
> driver.
>
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
>
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
>
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
>
> Series applies on v4.9-rc1
And FYI for anyone wanting to test, it needs a few config options
enabled[1] that are not (yet) part of davinci_all_defconfig. I'll
update the defconfig when I'm ready to send non-RFC patches.
Kevin
[1]
CONFIG_MEDIA_SUPPORT=y
CONFIG_MEDIA_CAMERA_SUPPORT=y
CONFIG_VIDEO_DEV=y
CONFIG_VIDEO_V4L2=y
CONFIG_VIDEO_V4L2_SUBDEV_API=y
CONFIG_V4L_PLATFORM_DRIVERS=y
CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE=y
# manually select codecs
CONFIG_MEDIA_SUBDRV_AUTOSELECT=n
# da850-lcdk
CONFIG_VIDEO_TVP514X=y
CONFIG_VIDEO_ADV7343=y
^ permalink raw reply
* SMR masking and PCI
From: Stuart Yoder @ 2016-10-28 17:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c39b785a-0f18-fc0a-ce08-7ebe3cfaf8c5@arm.com>
> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy at arm.com]
> Sent: Friday, October 28, 2016 11:17 AM
> To: Stuart Yoder <stuart.yoder@nxp.com>; Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel at lists.infradead.org; Will Deacon <will.deacon@arm.com>; Diana Madalina Craciun
> <diana.craciun@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; iommu at lists.linux-foundation.org
> Subject: Re: SMR masking and PCI
>
> Hi Stuart,
>
> On 27/10/16 18:10, Stuart Yoder wrote:
> > Hi Robin,
> >
> > A question about how the SMR masking defined in the arm,smmu binding
> > relates to the PCI iommu-map.
> >
> > The #iommu-cells property defines the number of cells an "IOMMU specifier"
> > takes and 2 is specified to be:
> >
> > SMMUs with stream matching support and complex masters
> > may use a value of 2, where the second cell represents
> > an SMR mask to combine with the ID in the first cell.
> >
> > An iommu-map entry is defined as:
> >
> > (rid-base,iommu,iommu-base,length)
> >
> > What seems to be currently missing in the iommu-map support is
> > the possibility the case where #iommu-cells=<2>.
>
> Indeed. The bindings have so far rather implicitly assumed the case of
> #{msi,iommu}-cells = 1, and the code has followed suit.
>
> > In this case iommu-base which is an IOMMU specifier should
> > occupy 2 cells. For example on an ls2085a we would want:
> >
> > iommu-map = <0x0 0x6 0x7 0x3ff 0x1
> > 0x100 0x6 0x8 0x3ff 0x1>;
> >
> > ...to mask our stream IDs to 10 bits.
> >
> > This should work in theory and comply with the bindings, no?
>
> In theory, but now consider:
>
> iommu-map = <0x0 0x6 0x7 0x3ff 0x2>;
>
> faced with ID 1. The input base is 0, the output base is the 2-cell
> value 0x7000003ff, so the final ID value should be 0x700000400, right?
No. The second cell as per the SMMU binding is the SMR mask...applied
by the SMMU before matching stream IDs.
In our case we want to mask off the upper TBU ID bits that the SMMU tags
onto the stream ID in our RID->SID LUT table.
RID = 0
SID in LUT and seen by SMMU = 7
SMMU-500 TBU appends bits, making SID something like: 0xC07
SMR mask of 0x3ff should be applied making the SID: 0x7
> > of_pci_map_rid() seems to have a hardcoded assumption that
> > each field in the map is 4 bytes.
>
> It does. I guess we should explicitly check that #{msi,iommu}-cells = 1
> on the target node, and maybe clarify in the binding that that cell
> should represent a plain linear ID value (although that's pretty obvious
> from context IMO).
>
> > (Also, I guess that msi-map is not affected by this since it
> > is not related to the IOMMU...but we do have common code
> > handling both maps.)
>
> I'd say it's definitely affected, since #msi-cells can equally be more
> than 1, and encodes an equally opaque value.
>
> It seems pretty reasonable to me that we could extend the binding to
> accommodate #cells > 1 iff length == 1. Mark?
I'm not following why the length matters.
> That said, is there a concrete need for this, i.e. do you actually have
> one device with a single requester ID, which maps to multiple output IDs
> (which differ only in the upper bits) in a non-predictable manner?
Devices behind an fsl-mc bus instance share the same stream ID (and GIC
ITS device ID) but may be behind different TBUs.
(see drivers/staging/fsl-mc/README.txt for overview info on fsl-mc)
So we could have a bus instance with all devices having a stream ID
of 0x5. But the TBU ID appending causes the TCU to see incoming
stream IDs from different devices with values of 0x105 and 0x205.
We need to get those upper bits masked off.
I don't see the above being an issue for PCI and platform devices. But
we don't want to expose TBU topography and IDs. Those are treated as
a microarchitecture detail in our SoC and are not even documented.
The SMMU programming model does not expose the TCU/TBU split.
Just treating the stream ID namespace as a clean linear space is
much easier to understand without having to grasp the existence
of TBUs at all. Masking SMRs keeps things clean.
As far as msi-map goes, I don't think there is an issue there. I
don't think the TBU appended bits are propagated to the GIC ITS
(as the device ID) so I don't think any masking is needed. Perhaps
you or Marc know.
It seems like perhaps what we need is a new argument passed to
of_pci_map_rid() that tells the function how many cells the
base iommu/msi identifier is. The caller supplies it.
Thanks,
Stuart
^ permalink raw reply
* [PATCH] staging: vc04_services: setup DMA and coherent mask
From: Michael Zoran @ 2016-10-28 17:11 UTC (permalink / raw)
To: linux-arm-kernel
Setting the DMA mask is optional on 32 bit but
is mandatory on 64 bit. Set the DMA mask and coherent
to force all DMA to be in the 32 bit address space.
This is considered a "good practice" and most drivers
already do this.
Signed-off-by: Michael Zoran <mzoran@crowfest.net>
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index a5afcc5..6fa2b5a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -97,6 +97,16 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
int slot_mem_size, frag_mem_size;
int err, irq, i;
+ /*
+ * Setting the DMA mask is necessary in the 64 bit environment.
+ * It isn't necessary in a 32 bit environment but is considered
+ * a good practice.
+ */
+ err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+ if (err < 0)
+ return err;
+
(void)of_property_read_u32(dev->of_node, "cache-line-size",
&g_cache_line_size);
g_fragments_size = 2 * g_cache_line_size;
--
2.10.1
^ permalink raw reply related
* [GIT PULL] Broadcom ARM64 Device Tree fixes for 4.9
From: Florian Fainelli @ 2016-10-28 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477331005-32163-1-git-send-email-f.fainelli@gmail.com>
On 10/24/2016 10:43 AM, Florian Fainelli wrote:
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
>
> Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>
> are available in the git repository at:
>
> http://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.9/devicetree-arm64-fixes
>
> for you to fetch changes up to 963d790468a2f581abf039b45edac79af5e16e55:
>
> arm64: dts: Updated NAND DT properties for NS2 SVK (2016-10-23 14:50:20 -0700)
>
> ----------------------------------------------------------------
> This pull request contains a single fix for Broadcom ARM64-based SoCs:
>
> - Ray adds the required bus width and OOB sector size properties to the
> Northstar 2 SVK reference board in order for the NAND controller to work
> properly
>
> ----------------------------------------------------------------
> Ray Jui (1):
> arm64: dts: Updated NAND DT properties for NS2 SVK
>
> arch/arm64/boot/dts/broadcom/ns2-svk.dts | 2 ++
> 1 file changed, 2 insertions(+)
Arnd, Kevin, Olof,
Can you take this change? Thanks!
--
Florian
^ permalink raw reply
* [PATCH v6 4/5] ARM: DTS: da850: Add cfgchip syscon node
From: Kevin Hilman @ 2016-10-28 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <09eb61b9-d66e-838e-2411-d787fda65096@ti.com>
Sekhar Nori <nsekhar@ti.com> writes:
> On Wednesday 26 October 2016 09:38 PM, David Lechner wrote:
>> On 10/25/2016 10:06 PM, David Lechner wrote:
>>> Add a syscon node for the SoC CFGCHIPn registers. This is needed for
>>> the new usb phy driver.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>> arch/arm/boot/dts/da850.dtsi | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index f79e1b9..6bbf20d 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -188,6 +188,10 @@
>>> };
>>>
>>> };
>>> + cfgchip: cfgchip at 1417c {
>>
>> I wonder if there is a more generic name instead of cfgchip at . Is there a
>> preferred generic name for syscon nodes?
>
> I did not find anything in ePAPR, but chip-controller might be more
> appropriate.
>
>>
>>> + compatible = "ti,da830-cfgchip", "syscon";
>
> Looks like we need "simple-mfd" too in the compatible list?
>
> I think we can also fold patch 5/5 into this patch and add the cfgchip
> along with USB phy child node included.
>
> If you respin the patch, I can drop 4/5 and 5/5 that I have queued and
> included the updated patch instead.
Sekhar, what's your opinion of having this syscon just for CFGCHIP* vs
a single syscon for the whole SYSCFG0 region.
The drivers/bus driver from Bartosz is also using SYSCFG0 registers, and
proposing a sysconf ro this region, but it will need to exclude the
CFGCHIPn registers if we also have this syscon.
I tend to think we should just have one for the whole SYSCFG0 which
this series could use.
Unfortunately, the PHY driver is already merged and it references the
syscon by compatible. The PHY driver should probably be fixed to find
its syscon by phandle, and then maybe we could move to a single syscon
for SYSCFG0?
Let us know your preference, I don't have a very strong feeling either
way, but since we're already part way down the path of the CFGCHIP
syscon, we should keep it and later migrate it to one for all of
SYSCFG0.
Kevin
^ permalink raw reply
* [PATCH 1/2] PCI: iproc: Support DT property for ignoring aborts when probing
From: Florian Fainelli @ 2016-10-28 17:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1b58db80-c9ff-d4d6-0df1-d80d1c03bc45@broadcom.com>
On 10/28/2016 09:58 AM, Ray Jui wrote:
> Hi Rafal,
>
> On 10/28/2016 8:31 AM, Rafa? Mi?ecki wrote:
>> On 20 April 2016 at 20:18, Ray Jui <ray.jui@broadcom.com> wrote:
>>> Hi Rafal/Florian/Arnd,
>>>
>>> After a couple days of email exchange with the ASIC team, I think I've
>>> figured out the behavior on all of the Broadcom SoCs that use this iProc
>>> PCIe controller.
>>>
>>> On NSP, Cygnus, and NS2:
>>> - There's an APB error enable register at offset 0xf40 from the iProc PCIe
>>> controller's base address. If one clears bit 0 (enabled by default after
>>> chip POR) of that register, one can stop this from being forwarded to "iProc
>>> host" as an APB error/external imprecise abort
>>> - I will submit a patch to the iProc PCIe driver to disable this error
>>> forwarding
>>>
>>> On NS:
>>> - Unfortunately, there's no such control register in NS. In other words, we
>>> cannot disable this error at the PCIe controller level
>>> - FSR code corresponds to external (bit[12] = '1'), read (bit[11] = '0'),
>>> imprecise abort (bits[10][3:0] = '1''0110'), i.e., external imprecise abort
>>> triggered by read access. Our ASIC team believes a read access to a
>>> non-exist APB register can also trigger an abort with the same FSR code.
>>> Note this is the tricky part, by registering an abort hook that skips this
>>> particular FSR, one has a chance of skipping other aborts triggered by
>>> accessing invalid APB registers. But given that this cannot be disabled for
>>> the PCIe controller NS, I'm not sure what approach we should take. Any
>>> thoughts?
>>
>> It's really late reply but I wanted to finally handle this problem.
>>
>> From Ray's e-mail it seems Northstar is the only platform requiring
>> this workaround. So we don't have to worry about arm64.
>
> Yes, Northstar is the only platform that requires this workaround. Even
> the arm32 platforms like NSP and Cygnus can disable unsupported request
> being forwarded as APB error. I've recently sent out a patch series to
> fix this for all other platforms, and sorry I should have included you
> in the email but I did not. I'll include you when revision 2 is sent out.
>
>>
>> We have two options then:
>> 1) Add workaround in arch/arm/mach-bcm/bcm_5301x.c
>> 2) Add workaround into built-in drivers/pci/host/pcie-iproc-fault.c
>
> How do you plan to implement pcie-iproc-fault.c? If it's similar to what
> you have now, then I think it fits more to bcm_5301x.c
I was going to suggest adding it to the PCIe driver so as to make it
localized there, but that seems like a better idea, in case the PCIe
driver is not built into the kernel, or as a module, it seems like a
nice thing to be able to clear the abort.
--
Florian
^ permalink raw reply
* [PATCH 1/2] of, numa: Add function to disable of_node_to_nid().
From: David Daney @ 2016-10-28 17:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028101905.GA6343@arm.com>
On 10/28/2016 03:19 AM, Will Deacon wrote:
> On Tue, Oct 25, 2016 at 02:31:00PM -0700, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> On arm64 NUMA kernels we can pass "numa=off" on the command line to
>> disable NUMA. A side effect of this is that kmalloc_node() calls to
>> non-zero nodes will crash the system with an OOPS:
>>
>> [ 0.000000] [<fffffc00081bba84>] __alloc_pages_nodemask+0xa4/0xe68
>> [ 0.000000] [<fffffc00082163a8>] new_slab+0xd0/0x57c
>> [ 0.000000] [<fffffc000821879c>] ___slab_alloc+0x2e4/0x514
>> [ 0.000000] [<fffffc000823882c>] __slab_alloc+0x48/0x58
>> [ 0.000000] [<fffffc00082195a0>] __kmalloc_node+0xd0/0x2e0
>> [ 0.000000] [<fffffc00081119b8>] __irq_domain_add+0x7c/0x164
>> [ 0.000000] [<fffffc0008b75d30>] its_probe+0x784/0x81c
>> [ 0.000000] [<fffffc0008b75e10>] its_init+0x48/0x1b0
>> .
>> .
>> .
>>
>> This is caused by code like this in kernel/irq/irqdomain.c
>>
>> domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
>> GFP_KERNEL, of_node_to_nid(of_node));
>>
>> When NUMA is disabled, the concept of a node is really undefined, so
>> of_node_to_nid() should unconditionally return NUMA_NO_NODE.
>>
>> Add __of_force_no_numa() to allow of_node_to_nid() to be forced to
>> return NUMA_NO_NODE.
>>
>> The follow on patch will call this new function from the arm64 numa
>> code.
>>
>> Reported-by: Gilbert Netzer <noname@pdc.kth.se>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>> drivers/of/of_numa.c | 15 +++++++++++++++
>> include/linux/of.h | 2 ++
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
>> index f63d4b0d..2212299 100644
>> --- a/drivers/of/of_numa.c
>> +++ b/drivers/of/of_numa.c
>> @@ -150,12 +150,27 @@ static int __init of_numa_parse_distance_map(void)
>> return ret;
>> }
>>
>> +static bool of_force_no_numa;
>> +
>> +void __of_force_no_numa(void)
>> +{
>> + of_force_no_numa = true;
>> +}
>> +
>> int of_node_to_nid(struct device_node *device)
>> {
>> struct device_node *np;
>> u32 nid;
>> int r = -ENODATA;
>>
>> + /*
>> + * If NUMA forced off, nodes are meaningless. Return
>> + * NUMA_NO_NODE so that any node specific memory allocations
>> + * can succeed from the default pool.
>> + */
>> + if (of_force_no_numa)
>> + return NUMA_NO_NODE;
>
> Why don't you just check if the nid you get back from the device is set in
> numa_nodes_parsed and return NUMA_NO_NODE if not?
numa_nodes_parsed is __initdata. Perhaps node_possible_map would be better.
I will try that.
David.
>
> Will
>
^ permalink raw reply
* [PATCH 1/2] PCI: iproc: Support DT property for ignoring aborts when probing
From: Ray Jui @ 2016-10-28 16:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACna6rxCdgyqFMU5-LD0rJn1s6voSiVvk1AGGnaykGZPMDbwPw@mail.gmail.com>
Hi Rafal,
On 10/28/2016 8:31 AM, Rafa? Mi?ecki wrote:
> On 20 April 2016 at 20:18, Ray Jui <ray.jui@broadcom.com> wrote:
>> Hi Rafal/Florian/Arnd,
>>
>> After a couple days of email exchange with the ASIC team, I think I've
>> figured out the behavior on all of the Broadcom SoCs that use this iProc
>> PCIe controller.
>>
>> On NSP, Cygnus, and NS2:
>> - There's an APB error enable register at offset 0xf40 from the iProc PCIe
>> controller's base address. If one clears bit 0 (enabled by default after
>> chip POR) of that register, one can stop this from being forwarded to "iProc
>> host" as an APB error/external imprecise abort
>> - I will submit a patch to the iProc PCIe driver to disable this error
>> forwarding
>>
>> On NS:
>> - Unfortunately, there's no such control register in NS. In other words, we
>> cannot disable this error at the PCIe controller level
>> - FSR code corresponds to external (bit[12] = '1'), read (bit[11] = '0'),
>> imprecise abort (bits[10][3:0] = '1''0110'), i.e., external imprecise abort
>> triggered by read access. Our ASIC team believes a read access to a
>> non-exist APB register can also trigger an abort with the same FSR code.
>> Note this is the tricky part, by registering an abort hook that skips this
>> particular FSR, one has a chance of skipping other aborts triggered by
>> accessing invalid APB registers. But given that this cannot be disabled for
>> the PCIe controller NS, I'm not sure what approach we should take. Any
>> thoughts?
>
> It's really late reply but I wanted to finally handle this problem.
>
> From Ray's e-mail it seems Northstar is the only platform requiring
> this workaround. So we don't have to worry about arm64.
Yes, Northstar is the only platform that requires this workaround. Even
the arm32 platforms like NSP and Cygnus can disable unsupported request
being forwarded as APB error. I've recently sent out a patch series to
fix this for all other platforms, and sorry I should have included you
in the email but I did not. I'll include you when revision 2 is sent out.
>
> We have two options then:
> 1) Add workaround in arch/arm/mach-bcm/bcm_5301x.c
> 2) Add workaround into built-in drivers/pci/host/pcie-iproc-fault.c
How do you plan to implement pcie-iproc-fault.c? If it's similar to what
you have now, then I think it fits more to bcm_5301x.c
>
> Do you have any preference about that?
>
Thanks,
Ray
^ permalink raw reply
* [PATCH v2 5/5] fpga manager: cyclone-ps-spi: make delay variable
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
The status pin may not show ready in the time described in the
Altetera manual. check the value several times before giving up
For the hardware I am working on, the status pin takes 250 us,
5 times as long as described by Altera.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
drivers/fpga/cyclone-ps-spi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
index 4b70d5c..c368223 100644
--- a/drivers/fpga/cyclone-ps-spi.c
+++ b/drivers/fpga/cyclone-ps-spi.c
@@ -20,6 +20,7 @@
#define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
-#define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
+#define FPGA_MIN_DELAY 50 /* min usecs to wait for config status */
+#define FPGA_MAX_DELAY 1000 /* max usecs to wait for config status */
struct cyclonespi_conf {
struct gpio_desc *config;
@@ -42,6 +43,7 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
const char *buf, size_t count)
{
struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+ int i;
if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
@@ -56,13 +58,14 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
}
gpiod_set_value(conf->config, 1);
- usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
- if (gpiod_get_value(conf->status) == 0) {
- dev_err(&mgr->dev, "Status pin not ready.\n");
- return -EIO;
+ for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
+ usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+ if (gpiod_get_value(conf->status))
+ return 0;
}
- return 0;
+ dev_err(&mgr->dev, "Status pin not ready.\n");
+ return -EIO;
}
static void rev_buf(void *buf, size_t len)
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/5] ARM: dts: imx6q-evi: support cyclonespi
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
Add support for Altera cyclone V FPGA connected to an spi port
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/boot/dts/imx6q-evi.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 6de21ff..bd0b85c 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -95,6 +95,15 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
status = "okay";
+
+ fpga_spi: cyclonespi at 0 {
+ compatible = "altr,cyclone-ps-spi-fpga-mgr";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ pinctrl-0 = <&pinctrl_fpgaspi>;
+ config-gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>;
+ status-gpio = <&gpio4 11 GPIO_ACTIVE_HIGH>;
+ };
};
&ecspi3 {
@@ -325,6 +334,13 @@
>;
};
+ pinctrl_fpgaspi: fpgaspigrp {
+ fsl,pins = <
+ MX6QDL_PAD_KEY_ROW1__GPIO4_IO09 0x1b0b0
+ MX6QDL_PAD_KEY_ROW2__GPIO4_IO11 0x1b0b0
+ >;
+ };
+
pinctrl_gpminand: gpminandgrp {
fsl,pins = <
MX6QDL_PAD_NANDF_CLE__NAND_CLE 0xb0b1
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/5] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
cyclone-ps-spi loads FPGA firmware over spi, using the "passive serial"
interface on Altera Cyclone FPGAS.
This is one of the simpler ways to set up an FPGA at runtime.
The signal interface is close to unidirectional spi with lsb first.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
drivers/fpga/Kconfig | 7 ++
drivers/fpga/Makefile | 1 +
drivers/fpga/cyclone-ps-spi.c | 172 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 180 insertions(+)
create mode 100644 drivers/fpga/cyclone-ps-spi.c
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index cd84934..2462707 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -13,6 +13,13 @@ config FPGA
if FPGA
+config FPGA_MGR_CYCLONE_PS_SPI
+ tristate "Altera Cyclone FPGA Passive Serial over SPI"
+ depends on SPI
+ help
+ FPGA manager driver support for Altera Cyclone using the
+ passive serial interface over SPI
+
config FPGA_MGR_SOCFPGA
tristate "Altera SOCFPGA FPGA Manager"
depends on ARCH_SOCFPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8d83fc6..8f93930 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -6,5 +6,6 @@
obj-$(CONFIG_FPGA) += fpga-mgr.o
# FPGA Manager Drivers
+obj-$(CONFIG_FPGA_MGR_CYCLONE_PS_SPI) += cyclone-ps-spi.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o
diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
new file mode 100644
index 0000000..4b70d5c
--- /dev/null
+++ b/drivers/fpga/cyclone-ps-spi.c
@@ -0,0 +1,172 @@
+/**
+ * Copyright (c) 2015 United Western Technologies, Corporation
+ *
+ * Joshua Clayton <stillcompiling@gmail.com>
+ *
+ * Manage Altera fpga firmware that is loaded over spi.
+ * Firmware must be in binary "rbf" format.
+ * Works on Cyclone V. Should work on cyclone series.
+ * May work on other Altera fpgas.
+ *
+ */
+
+#include <linux/bitrev.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+
+#define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
+#define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
+
+struct cyclonespi_conf {
+ struct gpio_desc *config;
+ struct gpio_desc *status;
+ struct spi_device *spi;
+};
+
+static const struct of_device_id of_ef_match[] = {
+ { .compatible = "altr,cyclone-ps-spi-fpga-mgr", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, of_ef_match);
+
+static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
+{
+ return mgr->state;
+}
+
+static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
+ const char *buf, size_t count)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+ if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
+ dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
+ return -EINVAL;
+ }
+
+ gpiod_set_value(conf->config, 0);
+ usleep_range(FPGA_RESET_TIME, FPGA_RESET_TIME + 20);
+ if (gpiod_get_value(conf->status) == 1) {
+ dev_err(&mgr->dev, "Status pin should be low.\n");
+ return -EIO;
+ }
+
+ gpiod_set_value(conf->config, 1);
+ usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+ if (gpiod_get_value(conf->status) == 0) {
+ dev_err(&mgr->dev, "Status pin not ready.\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static void rev_buf(void *buf, size_t len)
+{
+ u32 *fw32 = (u32 *)buf;
+ const u32 *fw_end = (u32 *)(buf + len);
+
+ /* set buffer to lsb first */
+ while (fw32 < fw_end) {
+ *fw32 = bitrev8x4(*fw32);
+ fw32++;
+ }
+}
+
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+ size_t count)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+ const char *fw_data = buf;
+ const char *fw_data_end = fw_data + count;
+
+ while (fw_data < fw_data_end) {
+ int ret;
+ size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+ rev_buf((void *)fw_data, stride);
+ ret = spi_write(conf->spi, fw_data, stride);
+ if (ret) {
+ dev_err(&mgr->dev, "spi error in firmware write: %d\n",
+ ret);
+ return ret;
+ }
+ fw_data += stride;
+ }
+
+ return 0;
+}
+
+static int cyclonespi_write_complete(struct fpga_manager *mgr, u32 flags)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+ if (gpiod_get_value(conf->status) == 0) {
+ dev_err(&mgr->dev, "Error during configuration.\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static const struct fpga_manager_ops cyclonespi_ops = {
+ .state = cyclonespi_state,
+ .write_init = cyclonespi_write_init,
+ .write = cyclonespi_write,
+ .write_complete = cyclonespi_write_complete,
+};
+
+static int cyclonespi_probe(struct spi_device *spi)
+{
+ struct cyclonespi_conf *conf = devm_kzalloc(&spi->dev, sizeof(*conf),
+ GFP_KERNEL);
+
+ if (!conf)
+ return -ENOMEM;
+
+ conf->spi = spi;
+ conf->config = devm_gpiod_get(&spi->dev, "config", GPIOD_OUT_LOW);
+ if (IS_ERR(conf->config)) {
+ dev_err(&spi->dev, "Failed to get config gpio: %ld\n",
+ PTR_ERR(conf->config));
+ return PTR_ERR(conf->config);
+ }
+
+ conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
+ if (IS_ERR(conf->status)) {
+ dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
+ PTR_ERR(conf->status));
+ return PTR_ERR(conf->status);
+ }
+
+ return fpga_mgr_register(&spi->dev,
+ "Altera Cyclone PS SPI FPGA Manager",
+ &cyclonespi_ops, conf);
+}
+
+static int cyclonespi_remove(struct spi_device *spi)
+{
+ fpga_mgr_unregister(&spi->dev);
+
+ return 0;
+}
+
+static struct spi_driver cyclonespi_driver = {
+ .driver = {
+ .name = "cyclone-ps-spi",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(of_ef_match),
+ },
+ .probe = cyclonespi_probe,
+ .remove = cyclonespi_remove,
+};
+
+module_spi_driver(cyclonespi_driver)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joshua Clayton <stillcompiling@gmail.com>");
+MODULE_DESCRIPTION("Module to load Altera FPGA firmware over spi");
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/5] doc: dt: add cyclone-spi binding document
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
Describe a cyclonei-ps-spi devicetree entry, required features
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
.../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
diff --git a/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
new file mode 100644
index 0000000..c942281
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
@@ -0,0 +1,23 @@
+Altera Cyclone Passive Serial SPI FPGA Manager
+
+Altera Cyclone FPGAs support a method of loading the bitstream over what is
+referred to as "passive serial".
+The passive serial link is not technically spi, and might require extra
+circuits in order to play nicely with other spi slaves on the same bus.
+
+See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
+
+Required properties:
+- compatible : should contain "altr,cyclone-ps-spi-fpga-mgr"
+- reg : spi slave id of the fpga
+- config-gpio : config pin (referred to as nCONFIG in the cyclone manual)
+- status-gpio : status pin (referred to as nSTATUS in the cyclone manual)
+
+Example:
+ fpga_spi: evi-fpga-spi at 0 {
+ compatible = "altr,cyclone-ps-spi-fpga-mgr";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ config-gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>;
+ status-gpio = <&gpio4 11 GPIO_ACTIVE_HIGH>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/5] lib: add bitrev8x4()
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
Add a function to reverse bytes within a 32 bit word.
This function is more efficient than using the 8 bit version when
iterating over an array
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/include/asm/bitrev.h | 5 +++++
include/linux/bitrev.h | 26 ++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
index ec291c3..6d2e9ca 100644
--- a/arch/arm/include/asm/bitrev.h
+++ b/arch/arm/include/asm/bitrev.h
@@ -17,4 +17,9 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return __arch_bitrev32((u32)x) >> 24;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+}
+
#endif
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index fb790b8..b1cfa1a 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -9,6 +9,7 @@
#define __bitrev32 __arch_bitrev32
#define __bitrev16 __arch_bitrev16
#define __bitrev8 __arch_bitrev8
+#define __bitrev8x4 __arch_bitrev8x4
#else
extern u8 const byte_rev_table[256];
@@ -27,6 +28,14 @@ static inline u32 __bitrev32(u32 x)
return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
}
+static inline u32 __bitrev8x4(u32 x)
+{
+ return(__bitrev8(x & 0xff) |
+ (__bitrev8((x >> 8) & 0xff) << 8) |
+ (__bitrev8((x >> 16) & 0xff) << 16) |
+ (__bitrev8((x >> 24) & 0xff) << 24));
+}
+
#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#define __constant_bitrev32(x) \
@@ -50,6 +59,15 @@ static inline u32 __bitrev32(u32 x)
__x; \
})
+#define __constant_bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4); \
+ __x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2); \
+ __x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1); \
+ __x; \
+})
+
#define __constant_bitrev8(x) \
({ \
u8 __x = x; \
@@ -75,6 +93,14 @@ static inline u32 __bitrev32(u32 x)
__bitrev16(__x); \
})
+#define bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __builtin_constant_p(__x) ? \
+ __constant_bitrev8x4(__x) : \
+ __bitrev8x4(__x); \
+})
+
#define bitrev8(x) \
({ \
u8 __x = x; \
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/4] Altera Cyclone Passive Serial SPI FPGA Manager
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
This series adds an FPGA manager for Altera cyclone FPGAs
that can program them using an spi port and a couple of gpios, using
Alteras passive serial protocol.
Changes from v1:
- Changed the name from cyclone-spi-fpga-mgr to cyclone-ps-spi-fpga-mgr
This name change was requested by Alan Tull, to be specific about which
programming method is being employed on the fpga.
- Changed the name of the reset-gpio to config-gpio to closer match the
way the pins are described in the Altera manual
- Moved MODULE_LICENCE, _AUTHOR, and _DESCRIPTION to the bottom
- Added a bitrev8x4() function to the bitrev headers and implemented ARM
const, runtime, and ARM specific faster versions (This may end up
needing to be a standalone patch)
- Moved the bitswapping into cyclonespi_write(), as requested.
This falls short of my desired generic lsb first spi support, but is a step
in that direction.
- Fixed whitespace problems introduced during refactoring
- Replaced magic number for initial delay with a descriptive macro
- Poll the fpga to see when it is ready rather than a fixed 1 ms sleep
Joshua Clayton (5):
lib: add bitrev8x4()
doc: dt: add cyclone-spi binding document
fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
ARM: dts: imx6q-evi: support cyclonespi
fpga manager: cyclone-ps-spi: make delay variable
.../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt | 23 +++
arch/arm/boot/dts/imx6q-evi.dts | 16 ++
arch/arm/include/asm/bitrev.h | 5 +
drivers/fpga/Kconfig | 7 +
drivers/fpga/Makefile | 1 +
drivers/fpga/cyclone-ps-spi.c | 175 +++++++++++++++++++++
include/linux/bitrev.h | 26 +++
7 files changed, 253 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
create mode 100644 drivers/fpga/cyclone-ps-spi.c
--
2.7.4
^ permalink raw reply
* [PATCH] fpga zynq: Check the bitstream for validity
From: Jason Gunthorpe @ 2016-10-28 16:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5ea0e77e-11c5-b4f7-00a9-9c5425ffac5a@suse.com>
On Fri, Oct 28, 2016 at 06:36:06PM +0200, Matthias Brugger wrote:
> Sure but we are checking here that the bitstream passed to the kernel is
> correct.
The intent to check if it *possible* that the bitstream is
correct. Correct means that DONE will assert after programming. A 4
byte bitstream will never assert DONE.
Arguably the threshold should be larger but we don't know what the
true minimum is.
> + /* All valid bitstreams are multiples of 32 bits */
> + if (!count || (count % 4) != 0)
> + return -EINVAL;
> +
Too clever for my taste.
Aside from this, is the general idea even OK? In my world I cannonize
the bitstream to start with the sync word, but others may not be doing
that.
I designed this patch based on the prior work to remove the
auto-detection, eg that the driver should be passed a canonized
bitstream.
The problem with the way it is now is how hard it is to figure out
what the right bitstream format should be. Clear instructions to
canonize by droping the header before the sync word and byteswap so
the sync word is in the given order is much clearer..
Jason
^ permalink raw reply
* [arm-platforms:kvm-arm64/fixes-4.9 2/3] arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio.c:483: undefined reference to `__aeabi_uldivmod'
From: kbuild test robot @ 2016-10-28 16:45 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm64/fixes-4.9
head: 3f3ed5826900f2f078417f8cf855c5977c3b28ea
commit: 465c188f7bc881c095448e8c2b16688b02ac5c3c [2/3] KVM: arm/arm64: vgic: Prevent access to invalid SPIs
config: arm-axm55xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 465c188f7bc881c095448e8c2b16688b02ac5c3c
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
arch/arm/kvm/built-in.o: In function `check_region':
>> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio.c:483: undefined reference to `__aeabi_uldivmod'
vim +483 arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio.c
467 case 4:
468 if (!((region->access_flags & VGIC_ACCESS_32bit) && !(addr & 3)))
469 return false;
470 break;
471 case 8:
472 if (!((region->access_flags & VGIC_ACCESS_64bit) && !(addr & 7)))
473 return false;
474 break;
475 default:
476 return false;
477 }
478
479 if (!region->bits_per_irq)
480 return true;
481
482 /* Do we access a non-allocated IRQ? */
> 483 return VGIC_ADDR_TO_INTID(addr, region->bits_per_irq) < nr_irqs;
484 }
485
486 static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
487 gpa_t addr, int len, void *val)
488 {
489 struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev);
490 const struct vgic_register_region *region;
491 unsigned long data = 0;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 19528 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161029/e5e15c7f/attachment-0001.gz>
^ permalink raw reply
* specifying order of /dev/mmcblk devices via device-tree?
From: Tim Harvey @ 2016-10-28 16:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028153755.GL5806@leverpostej>
On Fri, Oct 28, 2016 at 8:37 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Oct 28, 2016 at 08:23:04AM -0700, Tim Harvey wrote:
>> Greetings,
>>
>> I have an IMX6 board that has the following:
>> sdhc1: mmc0: sdio radio
>> sdhc2: mmc1: /dev/mmcblk1: microSD connector
>> sdhc3: mmc2: /dev/mmcblk2: on-board eMMC
>>
>> I would like to have sdhc3 registered as /dev/mmcblk0 and sdhc2
>> registered as /dev/mmcblk1 so that permanent storage is the first
>> mmcblk device as I think this is more intuitive however currently
>> these get instanced in the order they appear in the imx6qdl.dtsi
>> device-tree configuration and are not able to be mapped the way I want
>> them in my dts file.
>>
>> Is there a way, or if not is there a desire for a way, to specify the
>> order of /dev/mmcblk devices via device-tree?
>
> As with many other devices, there is no standard way of controlling the
> Linux enumeration (and given the ID space is shared with other dynamic
> devices it's not something that could generally work).
>
> These should be refererd to by UUID if possible.
>
> If not, we could cosider adding a by-dt-path or something like that.
>
> Thanks,
> Mark.
Mark / Javier/ Fabio,
Thanks - this is very useful.
Yes, I agree that using UUID's is the way to go and now I see how that
can be easily accessed via uboot 'part' command.
Regards,
Tim
^ permalink raw reply
* [PATCH] fpga: zynq-fpga: Delete not needed variable
From: Matthias Brugger @ 2016-10-28 16:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028112722.22837-1-mbrugger@suse.com>
On 28/10/16 13:27, Matthias Brugger wrote:
> Variable count is never changed in the write path,
> we don't need to save it for freeing the dma memory.
>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
This is obsolete, as Jason already has this in his patch [1].
Sorry for the noise.
[1] http://www.spinics.net/lists/arm-kernel/msg538868.html
> ---
> drivers/fpga/zynq-fpga.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index c2fb412..ffc2823 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -287,12 +287,10 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr,
> struct zynq_fpga_priv *priv;
> int err;
> char *kbuf;
> - size_t in_count;
> dma_addr_t dma_addr;
> u32 transfer_length;
> u32 intr_status;
>
> - in_count = count;
> priv = mgr->priv;
>
> kbuf = dma_alloc_coherent(priv->dev, count, &dma_addr, GFP_KERNEL);
> @@ -338,7 +336,7 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr,
> clk_disable(priv->clk);
>
> out_free:
> - dma_free_coherent(priv->dev, in_count, kbuf, dma_addr);
> + dma_free_coherent(priv->dev, count, kbuf, dma_addr);
>
> return err;
> }
>
^ permalink raw reply
* [PATCH] fpga zynq: Check the bitstream for validity
From: Matthias Brugger @ 2016-10-28 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028154819.GD10441@obsidianresearch.com>
On 28/10/16 17:48, Jason Gunthorpe wrote:
> On Fri, Oct 28, 2016 at 01:12:06PM +0200, Matthias Brugger wrote:
>>> Zynq is also only able to DMA dword quantities, so bitstreams must be
>>> a multiple of 4 bytes. This also fixes a DMA-past the end bug.
>>
>> The you can also fix the transfer_length calculation in zynq_fpga_ops_write,
>> as we don't allow buffers which are not a multiple of 4.
>
> Didn't I do that? Did you see something else to change in the dma
> part?
>
Sure you did, my mistake.
Matthias
^ permalink raw reply
* [PATCH] fpga zynq: Check the bitstream for validity
From: Matthias Brugger @ 2016-10-28 16:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028154740.GC10441@obsidianresearch.com>
On 28/10/16 17:47, Jason Gunthorpe wrote:
> On Fri, Oct 28, 2016 at 01:06:08PM +0200, Matthias Brugger wrote:
>
>> The only case we don't check is, if count == 0. If we check that here, we
>> can get rid of the count <= 4 check.
>
> You don't think
>
> if (count == 0 || buf[3] = 'x')
>
> looks weird and wrong? I do.
>
That wasn't what I meant. Apart it looks quite wrong, because when the
count is zero buf[3] points to anything but a valid value.
>>> The count <= 4 should stay here since it is primarily guarding against
>>> read past the buffer in the if.
>>
>> If you insist in doing this check, it should be count < 4, because we check
>> the first four elements of buf, or do I miss something?
>
> count = 4 and count = 0 are both invalid. A bitstream consisting of
> only the sync word is also going to fail programming.
>
> As Michal said, the actual min bitstream length is probably >> 50 bytes
>
Sure but we are checking here that the bitstream passed to the kernel is
correct. I was thinking of something like:
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index c2fb4120bd62..46a38772e7ee 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -184,12 +184,26 @@ static int zynq_fpga_ops_write_init(struct
fpga_manager *mgr, u32 flags,
priv = mgr->priv;
+ /* All valid bitstreams are multiples of 32 bits */
+ if (!count || (count % 4) != 0)
+ return -EINVAL;
+
^ permalink raw reply related
* [PATCH v2] phy: sun4i: check PMU presence when poking unknown bit of pmu
From: Icenowy Zheng @ 2016-10-28 16:27 UTC (permalink / raw)
To: linux-arm-kernel
Allwinner SoC's PHY 0, when used as OTG controller, have no pmu part.
The code that poke some unknown bit of PMU for H3/A64 didn't check
the PHY, and will cause kernel oops when PHY 0 is used.
This patch will check whether the pmu is not NULL before poking.
Fixes: b3e0d141ca9f (phy: sun4i: add support for A64 usb phy)
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/phy/phy-sun4i-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index b9342a2..fec34f5 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -264,7 +264,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
return ret;
}
- if (data->cfg->enable_pmu_unk1) {
+ if (phy->pmu && data->cfg->enable_pmu_unk1) {
val = readl(phy->pmu + REG_PMU_UNK1);
writel(val & ~2, phy->pmu + REG_PMU_UNK1);
}
--
2.10.1
^ permalink raw reply related
* [PATCH] video: ARM CLCD: fix Vexpress regression
From: Nicolae Rosia @ 2016-10-28 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476945992-5164-1-git-send-email-linus.walleij@linaro.org>
Thanks, works for me on qemu.
Tested-by: Nicolae Rosia <nicolae_rosia@mentor.com>
^ permalink raw reply
* SMR masking and PCI
From: Robin Murphy @ 2016-10-28 16:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <VI1PR0401MB26389CC8820C9654337B578E8DAA0@VI1PR0401MB2638.eurprd04.prod.outlook.com>
Hi Stuart,
On 27/10/16 18:10, Stuart Yoder wrote:
> Hi Robin,
>
> A question about how the SMR masking defined in the arm,smmu binding
> relates to the PCI iommu-map.
>
> The #iommu-cells property defines the number of cells an "IOMMU specifier"
> takes and 2 is specified to be:
>
> SMMUs with stream matching support and complex masters
> may use a value of 2, where the second cell represents
> an SMR mask to combine with the ID in the first cell.
>
> An iommu-map entry is defined as:
>
> (rid-base,iommu,iommu-base,length)
>
> What seems to be currently missing in the iommu-map support is
> the possibility the case where #iommu-cells=<2>.
Indeed. The bindings have so far rather implicitly assumed the case of
#{msi,iommu}-cells = 1, and the code has followed suit.
> In this case iommu-base which is an IOMMU specifier should
> occupy 2 cells. For example on an ls2085a we would want:
>
> iommu-map = <0x0 0x6 0x7 0x3ff 0x1
> 0x100 0x6 0x8 0x3ff 0x1>;
>
> ...to mask our stream IDs to 10 bits.
>
> This should work in theory and comply with the bindings, no?
In theory, but now consider:
iommu-map = <0x0 0x6 0x7 0x3ff 0x2>;
faced with ID 1. The input base is 0, the output base is the 2-cell
value 0x7000003ff, so the final ID value should be 0x700000400, right?
> of_pci_map_rid() seems to have a hardcoded assumption that
> each field in the map is 4 bytes.
It does. I guess we should explicitly check that #{msi,iommu}-cells = 1
on the target node, and maybe clarify in the binding that that cell
should represent a plain linear ID value (although that's pretty obvious
from context IMO).
> (Also, I guess that msi-map is not affected by this since it
> is not related to the IOMMU...but we do have common code
> handling both maps.)
I'd say it's definitely affected, since #msi-cells can equally be more
than 1, and encodes an equally opaque value.
It seems pretty reasonable to me that we could extend the binding to
accommodate #cells > 1 iff length == 1. Mark?
That said, is there a concrete need for this, i.e. do you actually have
one device with a single requester ID, which maps to multiple output IDs
(which differ only in the upper bits) in a non-predictable manner?
Robin.
>
> Thanks,
> Stuart
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox