* [PATCH RFC 00/12] tda998x updates
From: Jon Medhurst (Tixy) @ 2016-11-08 17:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108133411.GA31754@n2100.armlinux.org.uk>
On Tue, 2016-11-08 at 13:34 +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 08, 2016 at 01:32:15PM +0000, Russell King - ARM Linux wrote:
> > Unfortunately, my drm-tda998x-devel branch is slightly out of date with
> > these patches it's the original set of 10 patches. I've not pushed
> > these ones out to that branch yet, as I've three additional patches on
> > top of these which aren't "ready" for pushing out.
>
> Here's the delta between the branch and what I just posted:
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
[...]
I have a working setup for HDMI audio on Juno an would like to test this
series but am struggling to work out which patches to apply in what
order to what branch, can you be specific? (I've tried various
combinations of patches series from the list, drm-tda998x-devel, and the
diff you posted)
Thanks
--
Tixy
^ permalink raw reply
* [PATCH v2 0/8] Add support for monitoring guest TLB operations
From: Punit Agrawal @ 2016-11-08 17:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161026174148.17172-1-punit.agrawal@arm.com>
[ +PeterZ who should've been cc'd but doesn't show up in get_maintainers ]
Punit Agrawal <punit.agrawal@arm.com> writes:
> Hi,
>
> This is the fourth posting of this series. The biggest change compared
> to previous vesion is the addition of support for ARM hosts. With the
> addition of ARM support, the patchset is now more complete. Prior
> versions can be found at [0][1][2].
>
> I would particularly appreciate feedback from maintainers on the
> approach to tie the control of TLB monitoring with perf trace events
> (Patch 3 and 4) especially if there are any suggestions on avoiding
> (or reducing) the overhead of "perf trace" notifications.
>
> I looked at using regfunc/unregfunc tracepoint hooks but they don't
> include the event context. But the bigger problem was that the
> callbacks are only called on the first instance of simultaneously
> executing perf stat invocations.
I had a chance to discuss this patchset with SteveR and PeterZ at LPC
and both don't like the idea of introducing notifications on perf trace
event [un]registration.
Peter suggested using the PMU interface to expose this functionality. As
we want to monitor TLB instructions for for each VM, this will require
creating a PMU per-VM. The PMU events can be extended in the future if
monitoring of additional operations is supported and required.
I'll post the next version with the above changes - please chime-in if
you disagrees with this approach.
Thanks,
Punit
>
> Changelog and previous cover-letter follows.
>
> Changes:
>
> v1 -> v2
>
> * New (Patch 6) - Add support for trapping and emulating TLB
> operations to ARM hosts
> * Move common code to handle perf trace notifications to virt/kvm/arm
> * Move tracepoint to include/trace/events/kvm.h
> * Drop patch to introduce __tlbi helper as it is now merged
> * Reorder patches
>
> RFC v2 -> v1
> * Dropped the RFC tag
> * Patch 2 - Use VM thread group id for identification
> * Patch 4 - Update comment for clarity
> * Patch 6 - Add comment explaining switch to hype-role when VHE is enabled
> * Patch 7 - Add comment to clarify struct kvm_trace_hook
>
> RFC -> RFC v2
> * Patch 4 - Rename left-over TLBI macro to __TLBI
> * Patch 6 - Replace individual TLB operation emulation with
> invalidating all stage 1 TLB for the VM. TLB monitoring is expected
> to be a debug feature and performance is not critical.
>
> Although there are no PMU events to monitor TLB operations, ARMv8
> supports trapping guest TLB maintenance operations to the
> hypervisor. This trapping mechanism can be used to monitor the use of
> guest TLB instructions.
>
> As taking a trap for every TLB operation can have significant
> overhead, trapping should only be enabled -
>
> * on user request
> * for the VM of interest
>
> This patchset adds support to listen to perf trace event state change
> notifications. The notifications and associated context are then used
> to enable trapping of guest TLB operations when requested by the
> user. The trap handling generates trace events (kvm_tlb_invalidate)
> which can already be counted using existing perf trace functionality.
>
> With this patchset, 'perf' tool when attached to a VM process can be
> used to monitor the TLB operations. E.g., to monitor a VM with process
> id 4166 -
>
> # perf stat -e "kvm:kvm_tlb_invalidate" -p 4166
>
> Perform some operations in VM (running 'make -j 7' on the kernel
> sources in this instance). Breaking out of perf shows -
>
> Performance counter stats for process id '4166':
>
> 7,471,974 kvm:kvm_tlb_invalidate
>
> 374.235405282 seconds time elapsed
>
> Thanks,
> Punit
>
> [0] http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1210715.html
> [1] http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1224353.html
> [2] https://marc.info/?l=linux-kernel&m=147376184208258&w=2
>
> Mark Rutland (1):
> arm64/kvm: hyp: tlb: use __tlbi() helper
>
> Punit Agrawal (7):
> KVM: Track the pid of the VM process
> perf/trace: Add notification for perf trace events
> KVM: arm/arm64: Register perf trace event notifier
> KVM: Add event to trace tlb invalidations
> arm: KVM: Handle trappable TLB instructions
> arm64: KVM: Handle trappable TLB instructions
> KVM: arm/arm64: Enable selective trapping of TLB instructions
>
> arch/arm/include/asm/kvm_asm.h | 1 +
> arch/arm/include/asm/kvm_host.h | 8 ++
> arch/arm/kvm/Kconfig | 4 +
> arch/arm/kvm/Makefile | 1 +
> arch/arm/kvm/arm.c | 2 +
> arch/arm/kvm/coproc.c | 55 ++++++++++++++
> arch/arm/kvm/hyp/tlb.c | 33 ++++++++
> arch/arm64/include/asm/kvm_asm.h | 1 +
> arch/arm64/include/asm/kvm_host.h | 8 ++
> arch/arm64/kvm/Kconfig | 4 +
> arch/arm64/kvm/Makefile | 1 +
> arch/arm64/kvm/hyp/tlb.c | 87 +++++++++++++++++++--
> arch/arm64/kvm/sys_regs.c | 81 ++++++++++++++++++++
> include/linux/kvm_host.h | 1 +
> include/linux/trace_events.h | 3 +
> include/trace/events/kvm.h | 17 +++++
> kernel/trace/trace_event_perf.c | 24 ++++++
> virt/kvm/arm/perf_trace.c | 154 ++++++++++++++++++++++++++++++++++++++
> virt/kvm/kvm_main.c | 2 +
> 19 files changed, 481 insertions(+), 6 deletions(-)
> create mode 100644 virt/kvm/arm/perf_trace.c
^ permalink raw reply
* [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: Mark Rutland @ 2016-11-08 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2368890.jTbyGqYR0M@wuerfel>
On Tue, Nov 08, 2016 at 05:19:54PM +0100, Arnd Bergmann wrote:
> On Tuesday, November 8, 2016 11:49:53 AM CET Mark Rutland wrote:
> > My understanding of ISA (which may be flawed) is that it's not part of
> > the PCI host bridge, but rather on x86 it happens to share the IO space
> > with PCI.
>
> On normal systems, ISA or LPC are behind a PCI bridge device, which
> passes down both low addresses of I/O space and memory space.
Ok, so the use of those address spaces is an artifact of the ISA
controller being a device under the PCI host bridge.
Given we can have multiple domains, surely that implies we can have
multiple ISA controllers in general?
> > I believe that we could theoretically have multiple independent LPC/ISA
> > busses, as is possible with PCI on !x86 systems. If the current ISA code
> > assumes a singleton bus, I think that's something that needs to be fixed
> > up more generically.
> >
> > I don't see why we should need any architecture-specific code here. Why
> > can we not fix up the ISA bus code in drivers/of/address.c such that it
> > handles multiple ISA bus instances, and translates all sub-device
> > addresses relative to the specific bus instance?
>
> I think it is a relatively safe assumption that there is only one
> ISA bridge. A lot of old drivers hardcode PIO or memory addresses
> when talking to an ISA device, so having multiple instances is
> already problematic.
I'm worried that this might not be a safe assumption. Hardware these
days has a habit of pushing the boundaries of our expectations.
If we're going to assume that, I'd certainly want the kernel to verify
that it's true for all instanciated ISA/LPC devices. Otherwise, I can
imagine people relying on (or working around) that assumption in ACPI
tables and DTs, and that will be a nightmare (at best) to untangle in
future.
Thanks,
Mark.
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: John Garry @ 2016-11-08 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108164948.GG20591@arm.com>
On 08/11/2016 16:49, Will Deacon wrote:
> On Tue, Nov 08, 2016 at 04:33:44PM +0000, John Garry wrote:
>> On 08/11/2016 16:12, Will Deacon wrote:
>>> On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
>>>> +static inline void arm64_set_extops(struct extio_ops *ops)
>>>> +{
>>>> + if (ops)
>>>> + WRITE_ONCE(arm64_extio_ops, ops);
>>>
>>> Why does this need to be WRITE_ONCE? You don't have READ_ONCE on the reader
>>> side. Also, what if multiple drivers want to set different ops for distinct
>>> address ranges?
>>
>> I think that the idea here is that we only have possibly one master in the
>> system which offers indirectIO backend, so another one could not possibly
>> re-set this value.
>
> Why is that assumption valid, and why does WRITE_ONCE help there? It's not
> ONCE as in WARN_ONCE, more ONCE as in exactly-once-per-invocation.
It's only valid based on the inherent assumption that all indirectIO is
redirected to one backend master, i.e. LPC driver.
Anyway, right, I don't think that WRITE_ONCE is correct. Zhichang was
looking for something which would only allow the pointer to be written
once ever.
>
>>>> diff --git a/arch/arm64/kernel/extio.c b/arch/arm64/kernel/extio.c
>>>> new file mode 100644
>>>> index 0000000..647b3fa
>>>> --- /dev/null
>>>> +++ b/arch/arm64/kernel/extio.c
>>>> @@ -0,0 +1,27 @@
>>>> +/*
>>>> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
>>>> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public License
>>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>>> + */
>>>> +
>>>> +#include <linux/io.h>
>>>> +
>>>> +struct extio_ops *arm64_extio_ops;
>>>> +
>>>> +
>>>> +BUILD_EXTIO(b, u8)
>>>> +
>>>> +BUILD_EXTIO(w, u16)
>>>> +
>>>> +BUILD_EXTIO(l, u32)
>>>
>>> Is there no way to make this slightly more generic, so that it can be
>>> re-used elsewhere? For example, if struct extio_ops was common, then
>>> you could have the singleton (which maybe should be an interval tree?),
>>> type definition, setter function and the BUILD_EXTIO invocations
>>> somewhere generic, rather than squirelled away in the arch backend.
>>>
>> The concern would be that some architecture which uses generic higher-level
>> ISA accessor ops, but have IO space, could be affected.
>
> You're already adding a Kconfig symbol for this stuff, so you can keep
> that if you don't want it on other architectures. I'm just arguing that
> plumbing drivers directly into arch code via arm64_set_extops is not
> something I'm particularly fond of, especially when it looks like it
> could be avoided with a small amount of effort.
We'll check this.
Cheers,
John
>
> Will
>
> .
>
^ permalink raw reply
* [PATCH] arm: imx6qp: correct LDB clock inputs
From: Lucas Stach @ 2016-11-08 16:55 UTC (permalink / raw)
To: linux-arm-kernel
On i.MX6QP the LDB clock tree has changed to move the clk gate
before the divider, to prevent clock glitches propagating downstream.
A consequence of this change is that the clk divider is now the
parent of the LDB inputs. Reflect this change in the devicetree
to allow the LDB driver to properly configure the display clocks.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
arch/arm/boot/dts/imx6qp.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qp.dtsi b/arch/arm/boot/dts/imx6qp.dtsi
index 886dbf2eca49..caaa04036c8a 100644
--- a/arch/arm/boot/dts/imx6qp.dtsi
+++ b/arch/arm/boot/dts/imx6qp.dtsi
@@ -87,3 +87,13 @@
};
};
};
+
+&ldb {
+ clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>, <&clks IMX6QDL_CLK_LDB_DI1_SEL>,
+ <&clks IMX6QDL_CLK_IPU1_DI0_SEL>, <&clks IMX6QDL_CLK_IPU1_DI1_SEL>,
+ <&clks IMX6QDL_CLK_IPU2_DI0_SEL>, <&clks IMX6QDL_CLK_IPU2_DI1_SEL>,
+ <&clks IMX6QDL_CLK_LDB_DI0_PODF>, <&clks IMX6QDL_CLK_LDB_DI1_PODF>;
+ clock-names = "di0_pll", "di1_pll",
+ "di0_sel", "di1_sel", "di2_sel", "di3_sel",
+ "di0", "di1";
+};
--
2.10.1
^ permalink raw reply related
* [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers
From: Marcin Wojtas @ 2016-11-08 16:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108174851.2cd8efb4@free-electrons.com>
Hi Thomas,
2016-11-08 17:48 GMT+01:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Hello,
>
> On Tue, 8 Nov 2016 17:31:32 +0100, Marcin Wojtas wrote:
>> Enabling SPI controllers, which are attached to different busses
>> inside an SoC, may result in overlapping enumeration and cause
>> sysfs registration failure. Example log after enabling two
>> controllers on Armada 8040 SoC with same identifiers:
>>
>> [ 3.740415] sysfs: cannot create duplicate filename
>> '/class/spi_master/spi0'
>> [ 3.747510] ------------[ cut here ]------------
>> [ 3.752145] WARNING: at fs/sysfs/dir.c:31
>> [...]
>> [ 4.002299] orion_spi: probe of f4700600.spi failed with error -17
>>
>> spi-orion driver offers dedicated DT property ('cell-index'), that
>> allow setting unique identifiers. Recently added support for CP110-slave
>> HW block introduced two new SPI controllers' nodes with same ID as
>> ones from CP110-master.
>>
>> This commit fixes the issue by assigning different 'cell-index' values
>> for CP110-slave SPI controllers.
>>
>> Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
>> CP110 in Armada 8K")
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>
> It's sad that we need to hardcode those indexes in the Device Tree
> (which by no means are a description of the HW by the way), but that's
> what the SPI framework expects I believe.
Right. Devices are enumerated by generic code beginning from '0' on
each bus (AP806, CP110-master, CP110-slave) independently and
I didn't see any nice solution for it either.
> Therefore:
>
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
Thanks,
Marcin
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Will Deacon @ 2016-11-08 16:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8adfe182-4939-479d-6013-25ec40021b20@huawei.com>
On Tue, Nov 08, 2016 at 04:33:44PM +0000, John Garry wrote:
> On 08/11/2016 16:12, Will Deacon wrote:
> >On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> >>+static inline void arm64_set_extops(struct extio_ops *ops)
> >>+{
> >>+ if (ops)
> >>+ WRITE_ONCE(arm64_extio_ops, ops);
> >
> >Why does this need to be WRITE_ONCE? You don't have READ_ONCE on the reader
> >side. Also, what if multiple drivers want to set different ops for distinct
> >address ranges?
>
> I think that the idea here is that we only have possibly one master in the
> system which offers indirectIO backend, so another one could not possibly
> re-set this value.
Why is that assumption valid, and why does WRITE_ONCE help there? It's not
ONCE as in WARN_ONCE, more ONCE as in exactly-once-per-invocation.
> >>diff --git a/arch/arm64/kernel/extio.c b/arch/arm64/kernel/extio.c
> >>new file mode 100644
> >>index 0000000..647b3fa
> >>--- /dev/null
> >>+++ b/arch/arm64/kernel/extio.c
> >>@@ -0,0 +1,27 @@
> >>+/*
> >>+ * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
> >>+ * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify
> >>+ * it under the terms of the GNU General Public License version 2 as
> >>+ * published by the Free Software Foundation.
> >>+ *
> >>+ * This program is distributed in the hope that it will be useful,
> >>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>+ * GNU General Public License for more details.
> >>+ *
> >>+ * You should have received a copy of the GNU General Public License
> >>+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
> >>+ */
> >>+
> >>+#include <linux/io.h>
> >>+
> >>+struct extio_ops *arm64_extio_ops;
> >>+
> >>+
> >>+BUILD_EXTIO(b, u8)
> >>+
> >>+BUILD_EXTIO(w, u16)
> >>+
> >>+BUILD_EXTIO(l, u32)
> >
> >Is there no way to make this slightly more generic, so that it can be
> >re-used elsewhere? For example, if struct extio_ops was common, then
> >you could have the singleton (which maybe should be an interval tree?),
> >type definition, setter function and the BUILD_EXTIO invocations
> >somewhere generic, rather than squirelled away in the arch backend.
> >
> The concern would be that some architecture which uses generic higher-level
> ISA accessor ops, but have IO space, could be affected.
You're already adding a Kconfig symbol for this stuff, so you can keep
that if you don't want it on other architectures. I'm just arguing that
plumbing drivers directly into arch code via arm64_set_extops is not
something I'm particularly fond of, especially when it looks like it
could be avoided with a small amount of effort.
Will
^ permalink raw reply
* [PATCH] arm64: dts: marvell: Fix typo in label name on Armada 37xx
From: Thomas Petazzoni @ 2016-11-08 16:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108163216.24470-1-gregory.clement@free-electrons.com>
Hello,
On Tue, 8 Nov 2016 17:32:16 +0100, Gregory CLEMENT wrote:
> The label names of the peripheral clocks have a typo. Fix it before it is
> more widely used.
>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> - nb_perih_clk: nb-periph-clk at 13000{
> + nb_periph_clk: nb-periph-clk at 13000{
Space before {
> compatible = "marvell,armada-3700-periph-clock-nb";
> reg = <0x13000 0x100>;
> clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
> @@ -113,7 +113,7 @@
> #clock-cells = <1>;
> };
>
> - sb_perih_clk: sb-periph-clk at 18000{
> + sb_periph_clk: sb-periph-clk at 18000{
Ditto.
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers
From: Thomas Petazzoni @ 2016-11-08 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478622692-18441-1-git-send-email-mw@semihalf.com>
Hello,
On Tue, 8 Nov 2016 17:31:32 +0100, Marcin Wojtas wrote:
> Enabling SPI controllers, which are attached to different busses
> inside an SoC, may result in overlapping enumeration and cause
> sysfs registration failure. Example log after enabling two
> controllers on Armada 8040 SoC with same identifiers:
>
> [ 3.740415] sysfs: cannot create duplicate filename
> '/class/spi_master/spi0'
> [ 3.747510] ------------[ cut here ]------------
> [ 3.752145] WARNING: at fs/sysfs/dir.c:31
> [...]
> [ 4.002299] orion_spi: probe of f4700600.spi failed with error -17
>
> spi-orion driver offers dedicated DT property ('cell-index'), that
> allow setting unique identifiers. Recently added support for CP110-slave
> HW block introduced two new SPI controllers' nodes with same ID as
> ones from CP110-master.
>
> This commit fixes the issue by assigning different 'cell-index' values
> for CP110-slave SPI controllers.
>
> Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
> CP110 in Armada 8K")
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
It's sad that we need to hardcode those indexes in the Device Tree
(which by no means are a description of the HW by the way), but that's
what the SPI framework expects I believe. Therefore:
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v3 00/10] Add DT support for ohci-da8xx
From: Axel Haslam @ 2016-11-08 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161107203948.28324-1-ahaslam@baylibre.com>
Hi,
On Mon, Nov 7, 2016 at 9:39 PM, Axel Haslam <ahaslam@baylibre.com> wrote:
> The purpose of this patch series is to add DT support for the davinci
> ohci driver.
>
To make it easier to review. I will split the arch/arm and driver
patches into separate series.
Regards
Axel
> To be able to use device tree to probe the driver, we need to remove
> the platform callbacks that are handling vbus and over current.
>
> The first four patches prepare the stage by allowing to use a regulator
> instead of the callbacks.
>
> The next three patches convert the callback users to use a regulator
> instead and then remove the callbacks from the driver and platform code.
>
> Finally, we add device tree bindings and support in the driver.
>
> DEPENDENCIES:
> This series has depends on some patches currently under review
> but mostly accepted:
> 1. [PATCH 0/3] fix ohci phy name [1] (accepted)
> 2. [PATCH/RFC v2 0/3] regulator: handling of error conditions for usb drivers [2] (accepted)
> 3. [PATCH] gpio: davinci: Use unique labels for each gpio chip [3] (review pending)
>
> Also the current davinci baranches soon to be pulled to linux-next:
> davinci-for-v4.10/soc
> davinci-for-v4.10/dt
> davinci-for-v4.10/defconfig
> davinci-for-v4.10/cleanup
>
> A branch with all the dependencies can be found here [4].
>
> Changes form v2->v3
> * drop patches that have been integrated to arch/arm
> * drop regulator patches which will be integrated through regulator tree
> * use of the accepted regulator API to get over current status
> * better patch separation with the use of wrappers
>
> Changes from v1->v2
> * Rebased and added patch to make ohci a separate driver
> * Use a regulator instead of handling Gpios (David Lechner)
> * Add an over current mode to regulator framework
> * Fixed regulator is able to register for and over current irq
> * Added patch by Alexandre to remove build warnings
> * Moved global variables into private hcd structure.
>
> [1] https://lkml.org/lkml/2016/11/2/208
> [2] https://lkml.org/lkml/2016/11/3/188
> [3] http://www.spinics.net/lists/linux-gpio/msg17710.html
> [4] https://github.com/axelhaslamx/linux-axel/commits/ohci-da8xx-dt-v3
>
> Axel Haslam (10):
> USB: ohci: da8xx: use ohci priv data instead of globals
> USB: ohci: da8xx: Prepare to remove platform callbacks
> USB: ohci: da8xx: Allow a regulator to handle VBUS
> ARM: davinci: da830: Handle vbus with a regulator
> ARM: davinci: hawk: Remove vbus and over current gpios
> USB: ohci: da8xx: Remove ohci platform callbacks
> USB: ohci: da8xx: use a flag instead of mask for ocic
> USB: ohci: da8xx: Add devicetree bindings
> USB: ohci: da8xx: Allow probing from DT
> ARM: dts: da850: add usb device node
>
> .../devicetree/bindings/usb/ohci-da8xx.txt | 39 ++++
> arch/arm/boot/dts/da850-lcdk.dts | 8 +
> arch/arm/boot/dts/da850.dtsi | 8 +
> arch/arm/mach-davinci/board-da830-evm.c | 108 ++++-----
> arch/arm/mach-davinci/board-omapl138-hawk.c | 99 +-------
> arch/arm/mach-davinci/include/mach/da8xx.h | 2 +-
> arch/arm/mach-davinci/usb-da8xx.c | 3 +-
> drivers/usb/host/ohci-da8xx.c | 253 +++++++++++++++------
> include/linux/platform_data/usb-davinci.h | 20 --
> 9 files changed, 283 insertions(+), 257 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
>
> --
> 2.10.1
>
^ permalink raw reply
* [PATCH v2 0/2] arm64: fix the bugs found in the hugetlb test
From: Will Deacon @ 2016-11-08 16:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108140909.GF29087@e104818-lin.cambridge.arm.com>
On Tue, Nov 08, 2016 at 02:09:09PM +0000, Catalin Marinas wrote:
> On Tue, Nov 08, 2016 at 01:44:37PM +0800, Huang Shijie wrote:
> > (3) The test result in the Softiron and Juno-r1 boards:
> >
> > This detail test result shows below (both the "make func" & "make stress"):
> >
> > 4KB granule:
> >
> > 1.1) PTE + Contiguous bit : 4K x 16 = 64K (per huge page size)
> > Test result : PASS
> >
> > 1.2) PMD : 2M x 1 = 2M (per huge page size)
> > Test result : PASS
> >
> > 1.3) PMD + Contiguous bit : 2M x 16 = 32M (per huge page size)
> > Test result : PASS
> >
> > 64KB granule:
> >
> > 3.1) PTE + Contiguous bit : 64K x 32 = 2M (per huge page size)
> > Test result : PASS
> >
> > 3.2) PMD + Contiguous bit : 512M x 32 = 16G (per huge page size)
> > Test result : no hardware to support this test
>
> Don't we have support for single (non-contiguous) PMD huge page with 64K
> pages (512M per huge page)? I gave it a try and it seems to work (though
> without your patches applied ;)).
>
> > Huang Shijie (2):
> > arm64: hugetlb: remove the wrong pmd check in find_num_contig()
> > arm64: hugetlb: fix the wrong address for several functions
>
> For these patches:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
> I'm not sure whether Will plans to push them into 4.9. AFAICT, the
> contiguous huge pages never worked properly, so we may not count it as a
> regression but a new feature. If Will doesn't take them, I'll queue the
> patches for 4.10.
Right, given that it's never worked and the failure only seems to crop up
in synthetic testing, I think you can queue these for 4.10.
Will
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: John Garry @ 2016-11-08 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108161245.GD20591@arm.com>
On 08/11/2016 16:12, Will Deacon wrote:
> On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
>> For arm64, there is no I/O space as other architectural platforms, such as
>> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
>> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
>> known port addresses are used to control the corresponding target devices, for
>> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
>> normal MMIO mode in using.
>>
>> To drive these devices, this patch introduces a method named indirect-IO.
>> In this method the in/out pair in arch/arm64/include/asm/io.h will be
>> redefined. When upper layer drivers call in/out with those known legacy port
>> addresses to access the peripherals, the hooking functions corrresponding to
>> those target peripherals will be called. Through this way, those upper layer
>> drivers which depend on in/out can run on Hip06 without any changes.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>> ---
>> arch/arm64/Kconfig | 6 +++
>> arch/arm64/include/asm/extio.h | 94 ++++++++++++++++++++++++++++++++++++++++++
>> arch/arm64/include/asm/io.h | 29 +++++++++++++
>> arch/arm64/kernel/Makefile | 1 +
>> arch/arm64/kernel/extio.c | 27 ++++++++++++
>> 5 files changed, 157 insertions(+)
>> create mode 100644 arch/arm64/include/asm/extio.h
>> create mode 100644 arch/arm64/kernel/extio.c
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 969ef88..b44070b 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -163,6 +163,12 @@ config ARCH_MMAP_RND_COMPAT_BITS_MIN
>> config ARCH_MMAP_RND_COMPAT_BITS_MAX
>> default 16
>>
>> +config ARM64_INDIRECT_PIO
>> + bool "access peripherals with legacy I/O port"
>> + help
>> + Support special accessors for ISA I/O devices. This is needed for
>> + SoCs that do not support standard read/write for the ISA range.
>> +
>> config NO_IOPORT_MAP
>> def_bool y if !PCI
>>
>> diff --git a/arch/arm64/include/asm/extio.h b/arch/arm64/include/asm/extio.h
>> new file mode 100644
>> index 0000000..6ae0787
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/extio.h
>> @@ -0,0 +1,94 @@
>> +/*
>> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
>> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef __LINUX_EXTIO_H
>> +#define __LINUX_EXTIO_H
>> +
>> +struct extio_ops {
>> + unsigned long start;/* inclusive, sys io addr */
>> + unsigned long end;/* inclusive, sys io addr */
>> +
>> + u64 (*pfin)(void *devobj, unsigned long ptaddr, size_t dlen);
>> + void (*pfout)(void *devobj, unsigned long ptaddr, u32 outval,
>> + size_t dlen);
>> + u64 (*pfins)(void *devobj, unsigned long ptaddr, void *inbuf,
>> + size_t dlen, unsigned int count);
>> + void (*pfouts)(void *devobj, unsigned long ptaddr,
>> + const void *outbuf, size_t dlen,
>> + unsigned int count);
>> + void *devpara;
>> +};
>> +
>> +extern struct extio_ops *arm64_extio_ops;
>> +
>> +#define DECLARE_EXTIO(bw, type) \
>> +extern type in##bw(unsigned long addr); \
>> +extern void out##bw(type value, unsigned long addr); \
>> +extern void ins##bw(unsigned long addr, void *buffer, unsigned int count);\
>> +extern void outs##bw(unsigned long addr, const void *buffer, unsigned int count);
>> +
>> +#define BUILD_EXTIO(bw, type) \
>> +type in##bw(unsigned long addr) \
>> +{ \
>> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
>> + arm64_extio_ops->end < addr) \
>> + return read##bw(PCI_IOBASE + addr); \
>> + return arm64_extio_ops->pfin ? \
>> + arm64_extio_ops->pfin(arm64_extio_ops->devpara, \
>> + addr, sizeof(type)) : -1; \
>> +} \
>> + \
>> +void out##bw(type value, unsigned long addr) \
>> +{ \
>> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
>> + arm64_extio_ops->end < addr) \
>> + write##bw(value, PCI_IOBASE + addr); \
>> + else \
>> + if (arm64_extio_ops->pfout) \
>> + arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
>> + addr, value, sizeof(type)); \
>> +} \
>> + \
>> +void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
>> +{ \
>> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
>> + arm64_extio_ops->end < addr) \
>> + reads##bw(PCI_IOBASE + addr, buffer, count); \
>> + else \
>> + if (arm64_extio_ops->pfins) \
>> + arm64_extio_ops->pfins(arm64_extio_ops->devpara,\
>> + addr, buffer, sizeof(type), count); \
>> +} \
>> + \
>> +void outs##bw(unsigned long addr, const void *buffer, unsigned int count) \
>> +{ \
>> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
>> + arm64_extio_ops->end < addr) \
>> + writes##bw(PCI_IOBASE + addr, buffer, count); \
>> + else \
>> + if (arm64_extio_ops->pfouts) \
>> + arm64_extio_ops->pfouts(arm64_extio_ops->devpara,\
>> + addr, buffer, sizeof(type), count); \
>> +}
>> +
>> +static inline void arm64_set_extops(struct extio_ops *ops)
>> +{
>> + if (ops)
>> + WRITE_ONCE(arm64_extio_ops, ops);
>
> Why does this need to be WRITE_ONCE? You don't have READ_ONCE on the reader
> side. Also, what if multiple drivers want to set different ops for distinct
> address ranges?
I think that the idea here is that we only have possibly one master in
the system which offers indirectIO backend, so another one could not
possibly re-set this value.
>
>> +}
>> +
>> +#endif /* __LINUX_EXTIO_H*/
>> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
>> index 0bba427..136735d 100644
>> --- a/arch/arm64/include/asm/io.h
>> +++ b/arch/arm64/include/asm/io.h
>> @@ -31,6 +31,7 @@
>> #include <asm/early_ioremap.h>
>> #include <asm/alternative.h>
>> #include <asm/cpufeature.h>
>> +#include <asm/extio.h>
>>
>> #include <xen/xen.h>
>>
>> @@ -149,6 +150,34 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>
>> +
>> +/*
>> + * redefine the in(s)b/out(s)b for indirect-IO.
>> + */
>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>> +#define inb inb
>> +#define outb outb
>> +#define insb insb
>> +#define outsb outsb
>> +/* external declaration */
>> +DECLARE_EXTIO(b, u8)
>> +
>> +#define inw inw
>> +#define outw outw
>> +#define insw insw
>> +#define outsw outsw
>> +
>> +DECLARE_EXTIO(w, u16)
>> +
>> +#define inl inl
>> +#define outl outl
>> +#define insl insl
>> +#define outsl outsl
>> +
>> +DECLARE_EXTIO(l, u32)
>> +#endif
>> +
>> +
>> /*
>> * String version of I/O memory access operations.
>> */
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index 7d66bba..60e0482 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -31,6 +31,7 @@ arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \
>> sys_compat.o entry32.o
>> arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o
>> arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o
>> +arm64-obj-$(CONFIG_ARM64_INDIRECT_PIO) += extio.o
>> arm64-obj-$(CONFIG_ARM64_MODULE_PLTS) += module-plts.o
>> arm64-obj-$(CONFIG_PERF_EVENTS) += perf_regs.o perf_callchain.o
>> arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o
>> diff --git a/arch/arm64/kernel/extio.c b/arch/arm64/kernel/extio.c
>> new file mode 100644
>> index 0000000..647b3fa
>> --- /dev/null
>> +++ b/arch/arm64/kernel/extio.c
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
>> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/io.h>
>> +
>> +struct extio_ops *arm64_extio_ops;
>> +
>> +
>> +BUILD_EXTIO(b, u8)
>> +
>> +BUILD_EXTIO(w, u16)
>> +
>> +BUILD_EXTIO(l, u32)
>
> Is there no way to make this slightly more generic, so that it can be
> re-used elsewhere? For example, if struct extio_ops was common, then
> you could have the singleton (which maybe should be an interval tree?),
> type definition, setter function and the BUILD_EXTIO invocations
> somewhere generic, rather than squirelled away in the arch backend.
>
> Will
The concern would be that some architecture which uses generic
higher-level ISA accessor ops, but have IO space, could be affected.
John
>
> .
>
^ permalink raw reply
* [PATCH] arm64: dts: marvell: Fix typo in label name on Armada 37xx
From: Gregory CLEMENT @ 2016-11-08 16:32 UTC (permalink / raw)
To: linux-arm-kernel
The label names of the peripheral clocks have a typo. Fix it before it is
more widely used.
Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index c4762538ec01..693c36ac2c70 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -105,7 +105,7 @@
status = "disabled";
};
- nb_perih_clk: nb-periph-clk at 13000{
+ nb_periph_clk: nb-periph-clk at 13000{
compatible = "marvell,armada-3700-periph-clock-nb";
reg = <0x13000 0x100>;
clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
@@ -113,7 +113,7 @@
#clock-cells = <1>;
};
- sb_perih_clk: sb-periph-clk at 18000{
+ sb_periph_clk: sb-periph-clk at 18000{
compatible = "marvell,armada-3700-periph-clock-sb";
reg = <0x18000 0x100>;
clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
--
2.10.1
^ permalink raw reply related
* [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers
From: Marcin Wojtas @ 2016-11-08 16:31 UTC (permalink / raw)
To: linux-arm-kernel
Enabling SPI controllers, which are attached to different busses
inside an SoC, may result in overlapping enumeration and cause
sysfs registration failure. Example log after enabling two
controllers on Armada 8040 SoC with same identifiers:
[ 3.740415] sysfs: cannot create duplicate filename
'/class/spi_master/spi0'
[ 3.747510] ------------[ cut here ]------------
[ 3.752145] WARNING: at fs/sysfs/dir.c:31
[...]
[ 4.002299] orion_spi: probe of f4700600.spi failed with error -17
spi-orion driver offers dedicated DT property ('cell-index'), that
allow setting unique identifiers. Recently added support for CP110-slave
HW block introduced two new SPI controllers' nodes with same ID as
ones from CP110-master.
This commit fixes the issue by assigning different 'cell-index' values
for CP110-slave SPI controllers.
Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
CP110 in Armada 8K")
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index abb3fa2..d94d592 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -132,7 +132,7 @@
reg = <0x700600 0x50>;
#address-cells = <0x1>;
#size-cells = <0x0>;
- cell-index = <1>;
+ cell-index = <3>;
clocks = <&cps_syscon0 1 21>;
status = "disabled";
};
@@ -142,7 +142,7 @@
reg = <0x700680 0x50>;
#address-cells = <1>;
#size-cells = <0>;
- cell-index = <2>;
+ cell-index = <4>;
clocks = <&cps_syscon0 1 21>;
status = "disabled";
};
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: apq8064: add support to pm8821
From: Srinivas Kandagatla @ 2016-11-08 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478622577-20699-1-git-send-email-srinivas.kandagatla@linaro.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
arch/arm/boot/dts/qcom-apq8064.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 1dbe697..fde006c 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -627,6 +627,34 @@
clock-names = "core";
};
+ qcom,ssbi at c00000 {
+ compatible = "qcom,ssbi";
+ reg = <0x00c00000 0x1000>;
+ qcom,controller-type = "pmic-arbiter";
+
+ pmicintc2: pmic at 1 {
+ compatible = "qcom,pm8821";
+ interrupt-parent = <&tlmm_pinmux>;
+ interrupts = <76 8>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pm8821_mpps: mpps at 50 {
+
+ compatible = "qcom,pm8821-mpp", "qcom,ssbi-mpp";
+ reg = <0x50>;
+
+ interrupts = <24 1>, <25 1>, <26 1>,
+ <27 1>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };
+ };
+
qcom,ssbi at 500000 {
compatible = "qcom,ssbi";
reg = <0x00500000 0x1000>;
--
2.10.1
^ permalink raw reply related
* [PATCH 1/2] mfd: pm8921: add support to pm8821
From: Srinivas Kandagatla @ 2016-11-08 16:29 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds support to PM8821 PMIC and interrupt support.
PM8821 is companion device that supplements primary PMIC PM8921 IC.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
Tested this patch for MPP and IRQ functionality on IFC6410 and SD600 EVAL
board with mpps PM8821 and PM8921.
.../devicetree/bindings/mfd/qcom-pm8xxx.txt | 1 +
drivers/mfd/pm8921-core.c | 368 +++++++++++++++++++--
2 files changed, 340 insertions(+), 29 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
index 37a088f..8f1b4ec 100644
--- a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
+++ b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
@@ -11,6 +11,7 @@ voltages and other various functionality to Qualcomm SoCs.
Definition: must be one of:
"qcom,pm8058"
"qcom,pm8921"
+ "qcom,pm8821"
- #address-cells:
Usage: required
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index 0e3a2ea..28c2470 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -28,16 +28,26 @@
#include <linux/mfd/core.h>
#define SSBI_REG_ADDR_IRQ_BASE 0x1BB
-
-#define SSBI_REG_ADDR_IRQ_ROOT (SSBI_REG_ADDR_IRQ_BASE + 0)
-#define SSBI_REG_ADDR_IRQ_M_STATUS1 (SSBI_REG_ADDR_IRQ_BASE + 1)
-#define SSBI_REG_ADDR_IRQ_M_STATUS2 (SSBI_REG_ADDR_IRQ_BASE + 2)
-#define SSBI_REG_ADDR_IRQ_M_STATUS3 (SSBI_REG_ADDR_IRQ_BASE + 3)
-#define SSBI_REG_ADDR_IRQ_M_STATUS4 (SSBI_REG_ADDR_IRQ_BASE + 4)
-#define SSBI_REG_ADDR_IRQ_BLK_SEL (SSBI_REG_ADDR_IRQ_BASE + 5)
-#define SSBI_REG_ADDR_IRQ_IT_STATUS (SSBI_REG_ADDR_IRQ_BASE + 6)
-#define SSBI_REG_ADDR_IRQ_CONFIG (SSBI_REG_ADDR_IRQ_BASE + 7)
-#define SSBI_REG_ADDR_IRQ_RT_STATUS (SSBI_REG_ADDR_IRQ_BASE + 8)
+#define SSBI_PM8821_REG_ADDR_IRQ_BASE 0x100
+
+#define SSBI_REG_ADDR_IRQ_ROOT (0)
+#define SSBI_REG_ADDR_IRQ_M_STATUS1 (1)
+#define SSBI_REG_ADDR_IRQ_M_STATUS2 (2)
+#define SSBI_REG_ADDR_IRQ_M_STATUS3 (3)
+#define SSBI_REG_ADDR_IRQ_M_STATUS4 (4)
+#define SSBI_REG_ADDR_IRQ_BLK_SEL (5)
+#define SSBI_REG_ADDR_IRQ_IT_STATUS (6)
+#define SSBI_REG_ADDR_IRQ_CONFIG (7)
+#define SSBI_REG_ADDR_IRQ_RT_STATUS (8)
+
+#define PM8821_TOTAL_IRQ_MASTERS 2
+#define PM8821_BLOCKS_PER_MASTER 7
+#define PM8821_IRQ_MASTER1_SET 0x01
+#define PM8821_IRQ_CLEAR_OFFSET 0x01
+#define PM8821_IRQ_RT_STATUS_OFFSET 0x0f
+#define PM8821_IRQ_MASK_REG_OFFSET 0x08
+#define SSBI_REG_ADDR_IRQ_MASTER0 0x30
+#define SSBI_REG_ADDR_IRQ_MASTER1 0xb0
#define PM_IRQF_LVL_SEL 0x01 /* level select */
#define PM_IRQF_MASK_FE 0x02 /* mask falling edge */
@@ -54,30 +64,41 @@
#define REG_HWREV_2 0x0E8 /* PMIC4 revision 2 */
#define PM8921_NR_IRQS 256
+#define PM8821_NR_IRQS 112
struct pm_irq_chip {
struct regmap *regmap;
spinlock_t pm_irq_lock;
struct irq_domain *irqdomain;
+ unsigned int irq_reg_base;
unsigned int num_irqs;
unsigned int num_blocks;
unsigned int num_masters;
u8 config[0];
};
+struct pm8xxx_data {
+ int num_irqs;
+ unsigned int irq_reg_base;
+ const struct irq_domain_ops *irq_domain_ops;
+ void (*irq_handler)(struct irq_desc *desc);
+};
+
static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, unsigned int bp,
unsigned int *ip)
{
int rc;
spin_lock(&chip->pm_irq_lock);
- rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+ rc = regmap_write(chip->regmap,
+ chip->irq_reg_base + SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
if (rc) {
pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
goto bail;
}
- rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
+ rc = regmap_read(chip->regmap,
+ chip->irq_reg_base + SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
if (rc)
pr_err("Failed Reading Status rc=%d\n", rc);
bail:
@@ -91,14 +112,16 @@ pm8xxx_config_irq(struct pm_irq_chip *chip, unsigned int bp, unsigned int cp)
int rc;
spin_lock(&chip->pm_irq_lock);
- rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+ rc = regmap_write(chip->regmap,
+ chip->irq_reg_base + SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
if (rc) {
pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
goto bail;
}
cp |= PM_IRQF_WRITE;
- rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_CONFIG, cp);
+ rc = regmap_write(chip->regmap,
+ chip->irq_reg_base + SSBI_REG_ADDR_IRQ_CONFIG, cp);
if (rc)
pr_err("Failed Configuring IRQ rc=%d\n", rc);
bail:
@@ -137,8 +160,8 @@ static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
unsigned int blockbits;
int block_number, i, ret = 0;
- ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_M_STATUS1 + master,
- &blockbits);
+ ret = regmap_read(chip->regmap, chip->irq_reg_base +
+ SSBI_REG_ADDR_IRQ_M_STATUS1 + master, &blockbits);
if (ret) {
pr_err("Failed to read master %d ret=%d\n", master, ret);
return ret;
@@ -165,7 +188,8 @@ static void pm8xxx_irq_handler(struct irq_desc *desc)
chained_irq_enter(irq_chip, desc);
- ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_ROOT, &root);
+ ret = regmap_read(chip->regmap,
+ chip->irq_reg_base + SSBI_REG_ADDR_IRQ_ROOT, &root);
if (ret) {
pr_err("Can't read root status ret=%d\n", ret);
return;
@@ -182,6 +206,122 @@ static void pm8xxx_irq_handler(struct irq_desc *desc)
chained_irq_exit(irq_chip, desc);
}
+static int pm8821_read_master_irq(const struct pm_irq_chip *chip,
+ int m, unsigned int *master)
+{
+ unsigned int base;
+
+ if (!m)
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
+ else
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
+
+ return regmap_read(chip->regmap, base, master);
+}
+
+static int pm8821_read_block_irq(struct pm_irq_chip *chip, int master,
+ u8 block, unsigned int *bits)
+{
+ int rc;
+
+ unsigned int base;
+
+ if (!master)
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
+ else
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
+
+ spin_lock(&chip->pm_irq_lock);
+
+ rc = regmap_read(chip->regmap, base + block, bits);
+ if (rc)
+ pr_err("Failed Reading Status rc=%d\n", rc);
+
+ spin_unlock(&chip->pm_irq_lock);
+ return rc;
+}
+
+static int pm8821_irq_block_handler(struct pm_irq_chip *chip,
+ int master_number, int block)
+{
+ int pmirq, irq, i, ret;
+ unsigned int bits;
+
+ ret = pm8821_read_block_irq(chip, master_number, block, &bits);
+ if (ret) {
+ pr_err("Failed reading %d block ret=%d", block, ret);
+ return ret;
+ }
+ if (!bits) {
+ pr_err("block bit set in master but no irqs: %d", block);
+ return 0;
+ }
+
+ /* Convert block offset to global block number */
+ block += (master_number * PM8821_BLOCKS_PER_MASTER) - 1;
+
+ /* Check IRQ bits */
+ for (i = 0; i < 8; i++) {
+ if (bits & BIT(i)) {
+ pmirq = block * 8 + i;
+ irq = irq_find_mapping(chip->irqdomain, pmirq);
+ generic_handle_irq(irq);
+ }
+ }
+
+ return 0;
+}
+
+static int pm8821_irq_read_master(struct pm_irq_chip *chip,
+ int master_number, u8 master_val)
+{
+ int ret = 0;
+ int block;
+
+ for (block = 1; block < 8; block++) {
+ if (master_val & BIT(block)) {
+ ret |= pm8821_irq_block_handler(chip,
+ master_number, block);
+ }
+ }
+
+ return ret;
+}
+
+static void pm8821_irq_handler(struct irq_desc *desc)
+{
+ struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
+ struct irq_chip *irq_chip = irq_desc_get_chip(desc);
+ int ret;
+ unsigned int master;
+
+ chained_irq_enter(irq_chip, desc);
+ /* check master 0 */
+ ret = pm8821_read_master_irq(chip, 0, &master);
+ if (ret) {
+ pr_err("Failed to re:Qad master 0 ret=%d\n", ret);
+ return;
+ }
+
+ if (master & ~PM8821_IRQ_MASTER1_SET)
+ pm8821_irq_read_master(chip, 0, master);
+
+ /* check master 1 */
+ if (!(master & PM8821_IRQ_MASTER1_SET))
+ goto done;
+
+ ret = pm8821_read_master_irq(chip, 1, &master);
+ if (ret) {
+ pr_err("Failed to read master 1 ret=%d\n", ret);
+ return;
+ }
+
+ pm8821_irq_read_master(chip, 1, master);
+
+done:
+ chained_irq_exit(irq_chip, desc);
+}
+
static void pm8xxx_irq_mask_ack(struct irq_data *d)
{
struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
@@ -254,13 +394,15 @@ static int pm8xxx_irq_get_irqchip_state(struct irq_data *d,
irq_bit = pmirq % 8;
spin_lock(&chip->pm_irq_lock);
- rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
+ rc = regmap_write(chip->regmap, chip->irq_reg_base +
+ SSBI_REG_ADDR_IRQ_BLK_SEL, block);
if (rc) {
pr_err("Failed Selecting Block %d rc=%d\n", block, rc);
goto bail;
}
- rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
+ rc = regmap_read(chip->regmap, chip->irq_reg_base +
+ SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
if (rc) {
pr_err("Failed Reading Status rc=%d\n", rc);
goto bail;
@@ -299,6 +441,151 @@ static const struct irq_domain_ops pm8xxx_irq_domain_ops = {
.map = pm8xxx_irq_domain_map,
};
+static void pm8821_irq_mask_ack(struct irq_data *d)
+{
+ struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+ unsigned int base, pmirq = irqd_to_hwirq(d);
+ u8 block, master;
+ int irq_bit, rc;
+
+ block = pmirq / 8;
+ master = block / PM8821_BLOCKS_PER_MASTER;
+ irq_bit = pmirq % 8;
+ block %= PM8821_BLOCKS_PER_MASTER;
+
+ if (!master)
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
+ else
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
+
+ spin_lock(&chip->pm_irq_lock);
+ rc = regmap_update_bits(chip->regmap,
+ base + PM8821_IRQ_MASK_REG_OFFSET + block,
+ BIT(irq_bit), BIT(irq_bit));
+
+ if (rc) {
+ pr_err("Failed to read/write mask IRQ:%d rc=%d\n", pmirq, rc);
+ goto fail;
+ }
+
+ rc = regmap_update_bits(chip->regmap,
+ base + PM8821_IRQ_CLEAR_OFFSET + block,
+ BIT(irq_bit), BIT(irq_bit));
+
+ if (rc) {
+ pr_err("Failed to read/write IT_CLEAR IRQ:%d rc=%d\n",
+ pmirq, rc);
+ }
+
+fail:
+ spin_unlock(&chip->pm_irq_lock);
+}
+
+static void pm8821_irq_unmask(struct irq_data *d)
+{
+ struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+ unsigned int base, pmirq = irqd_to_hwirq(d);
+ int irq_bit, rc;
+ u8 block, master;
+
+ block = pmirq / 8;
+ master = block / PM8821_BLOCKS_PER_MASTER;
+ irq_bit = pmirq % 8;
+ block %= PM8821_BLOCKS_PER_MASTER;
+
+ if (!master)
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
+ else
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
+
+ spin_lock(&chip->pm_irq_lock);
+
+ rc = regmap_update_bits(chip->regmap,
+ base + PM8821_IRQ_MASK_REG_OFFSET + block,
+ BIT(irq_bit), ~BIT(irq_bit));
+
+ if (rc)
+ pr_err("Failed to read/write unmask IRQ:%d rc=%d\n", pmirq, rc);
+
+ spin_unlock(&chip->pm_irq_lock);
+}
+
+static int pm8821_irq_set_type(struct irq_data *d, unsigned int flow_type)
+{
+
+ /*
+ * PM8821 IRQ controller does not have explicit software support for
+ * IRQ flow type.
+ */
+ return 0;
+}
+
+static int pm8821_irq_get_irqchip_state(struct irq_data *d,
+ enum irqchip_irq_state which,
+ bool *state)
+{
+ struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+ int pmirq, rc;
+ u8 block, irq_bit, master;
+ unsigned int bits;
+ unsigned int base;
+ unsigned long flags;
+
+ pmirq = irqd_to_hwirq(d);
+
+ block = pmirq / 8;
+ master = block / PM8821_BLOCKS_PER_MASTER;
+ irq_bit = pmirq % 8;
+ block %= PM8821_BLOCKS_PER_MASTER;
+
+ if (!master)
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
+ else
+ base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
+
+ spin_lock_irqsave(&chip->pm_irq_lock, flags);
+
+ rc = regmap_read(chip->regmap,
+ base + PM8821_IRQ_RT_STATUS_OFFSET + block, &bits);
+ if (rc) {
+ pr_err("Failed Reading Status rc=%d\n", rc);
+ goto bail_out;
+ }
+
+ *state = !!(bits & BIT(irq_bit));
+
+bail_out:
+ spin_unlock_irqrestore(&chip->pm_irq_lock, flags);
+
+ return rc;
+}
+
+static struct irq_chip pm8821_irq_chip = {
+ .name = "pm8821",
+ .irq_mask_ack = pm8821_irq_mask_ack,
+ .irq_unmask = pm8821_irq_unmask,
+ .irq_set_type = pm8821_irq_set_type,
+ .irq_get_irqchip_state = pm8821_irq_get_irqchip_state,
+ .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE,
+};
+
+static int pm8821_irq_domain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ struct pm_irq_chip *chip = d->host_data;
+
+ irq_set_chip_and_handler(irq, &pm8821_irq_chip, handle_level_irq);
+ irq_set_chip_data(irq, chip);
+ irq_set_noprobe(irq);
+
+ return 0;
+}
+
+static const struct irq_domain_ops pm8821_irq_domain_ops = {
+ .xlate = irq_domain_xlate_twocell,
+ .map = pm8821_irq_domain_map,
+};
+
static const struct regmap_config ssbi_regmap_config = {
.reg_bits = 16,
.val_bits = 8,
@@ -308,10 +595,25 @@ static const struct regmap_config ssbi_regmap_config = {
.reg_write = ssbi_reg_write
};
+static const struct pm8xxx_data pm8xxx_data = {
+ .num_irqs = PM8921_NR_IRQS,
+ .irq_reg_base = SSBI_REG_ADDR_IRQ_BASE,
+ .irq_domain_ops = &pm8xxx_irq_domain_ops,
+ .irq_handler = pm8xxx_irq_handler,
+};
+
+static const struct pm8xxx_data pm8821_data = {
+ .num_irqs = PM8821_NR_IRQS,
+ .irq_reg_base = SSBI_PM8821_REG_ADDR_IRQ_BASE,
+ .irq_domain_ops = &pm8821_irq_domain_ops,
+ .irq_handler = pm8821_irq_handler,
+};
+
static const struct of_device_id pm8921_id_table[] = {
- { .compatible = "qcom,pm8018", },
- { .compatible = "qcom,pm8058", },
- { .compatible = "qcom,pm8921", },
+ { .compatible = "qcom,pm8018", .data = &pm8xxx_data},
+ { .compatible = "qcom,pm8058", .data = &pm8xxx_data},
+ { .compatible = "qcom,pm8821", .data = &pm8821_data},
+ { .compatible = "qcom,pm8921", .data = &pm8xxx_data},
{ }
};
MODULE_DEVICE_TABLE(of, pm8921_id_table);
@@ -319,11 +621,17 @@ MODULE_DEVICE_TABLE(of, pm8921_id_table);
static int pm8921_probe(struct platform_device *pdev)
{
struct regmap *regmap;
+ const struct pm8xxx_data *data;
int irq, rc;
unsigned int val;
u32 rev;
struct pm_irq_chip *chip;
- unsigned int nirqs = PM8921_NR_IRQS;
+
+ data = of_match_node(pm8921_id_table, pdev->dev.of_node)->data;
+ if (!data) {
+ dev_err(&pdev->dev, "No matching driver data found\n");
+ return -EINVAL;
+ }
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -354,25 +662,27 @@ static int pm8921_probe(struct platform_device *pdev)
rev |= val << BITS_PER_BYTE;
chip = devm_kzalloc(&pdev->dev, sizeof(*chip) +
- sizeof(chip->config[0]) * nirqs,
- GFP_KERNEL);
+ sizeof(chip->config[0]) * data->num_irqs,
+ GFP_KERNEL);
if (!chip)
return -ENOMEM;
platform_set_drvdata(pdev, chip);
chip->regmap = regmap;
- chip->num_irqs = nirqs;
+ chip->num_irqs = data->num_irqs;
+ chip->irq_reg_base = data->irq_reg_base;
chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
spin_lock_init(&chip->pm_irq_lock);
- chip->irqdomain = irq_domain_add_linear(pdev->dev.of_node, nirqs,
- &pm8xxx_irq_domain_ops,
+ chip->irqdomain = irq_domain_add_linear(pdev->dev.of_node,
+ data->num_irqs,
+ data->irq_domain_ops,
chip);
if (!chip->irqdomain)
return -ENODEV;
- irq_set_chained_handler_and_data(irq, pm8xxx_irq_handler, chip);
+ irq_set_chained_handler_and_data(irq, data->irq_handler, chip);
irq_set_irq_wake(irq, 1);
rc = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
--
2.10.1
^ permalink raw reply related
* [PATCH 4/6] clk: stm32f4: Add I2S clock
From: Gabriel Fernandez @ 2016-11-08 16:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <08ad0f8c-bcdf-4835-9f07-167e129604bd@linaro.org>
On 11/07/2016 03:14 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, gabriel.fernandez at st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> This patch introduces I2S clock for stm32f4 soc.
>> The I2S clock could be derived from an external clock or from pll-i2s
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>> ---
>> drivers/clk/clk-stm32f4.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index 5fa5d51..b7cb359 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -216,6 +216,7 @@ enum {
>> SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
>> PLL_VCO_I2S, PLL_VCO_SAI,
>> CLK_LCD,
>> + CLK_I2S,
>
> Sorry, this has just clicked and it applies to most of the other
> patches, but adding things to this list effectively extends the clock
> bindings (i.e. the list of valid "other" clocks access with a primary
> index of 1).
>
> This list if a list of "arbitrary" constants by which DT periphericals
> can be linked to specific clocks.
>
> So...
>
> 1) If a clock is introduced here we should update the clock binding
> documentations.
>
> 2) If no peripheral can connect to the clock (because it is internal
> to the clock gen logic and peripherals must connect to the gated
> version) it should not be included in this enum.
>
> 3) I failed to mention this when the four undocumented clocks
> (LSI, LSE, HSE_RTC and RTC) were added.
>
> 4) I *should* have added a comment explaining the above to the code.
>
>
ok i agree
>> END_PRIMARY_CLK
>> };
>>
>> @@ -967,6 +968,8 @@ static struct clk_hw *stm32_register_cclk(struct
>> device *dev, const char *name,
>>
>> static const char *sdmux_parents[2] = { "pll48", "sys" };
>>
>> +static const char *i2s_parents[2] = { "plli2s-r", NULL };
>> +
>> struct stm32f4_clk_data {
>> const struct stm32f4_gate_data *gates_data;
>> const u64 *gates_map;
>> @@ -1005,7 +1008,7 @@ struct stm32f4_clk_data {
>>
>> static void __init stm32f4_rcc_init(struct device_node *np)
>> {
>> - const char *hse_clk;
>> + const char *hse_clk, *i2s_in_clk;
>> int n;
>> const struct of_device_id *match;
>> const struct stm32f4_clk_data *data;
>> @@ -1038,6 +1041,7 @@ static void __init stm32f4_rcc_init(struct
>> device_node *np)
>> stm32f4_gate_map = data->gates_map;
>>
>> hse_clk = of_clk_get_parent_name(np, 0);
>> + i2s_in_clk = of_clk_get_parent_name(np, 1);
>
> Again this looks like a change to the DT bindings.
>
ok
> Also does the code work if i2s_in_clk is NULL or as you hoping to get
> away with a not-backwards compatible change?
>
>
yes it works if i2s_in_clk is NULL.
BR
Gabriel
> Daniel.
>
>
>>
>> clk_register_fixed_rate_with_accuracy(NULL, "hsi", NULL, 0,
>> 16000000, 160000);
>> @@ -1053,6 +1057,12 @@ static void __init stm32f4_rcc_init(struct
>> device_node *np)
>> clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll(pllsrc,
>> &data->pll_data[2], &stm32f4_clk_lock);
>>
>> + i2s_parents[1] = i2s_in_clk;
>> +
>> + clks[CLK_I2S] = clk_hw_register_mux_table(NULL, "i2s",
>> + i2s_parents, ARRAY_SIZE(i2s_parents), 0,
>> + base + STM32F4_RCC_CFGR, 23, 1, 0, NULL,
>> + &stm32f4_clk_lock);
>> sys_parents[1] = hse_clk;
>> clk_register_mux_table(
>> NULL, "sys", sys_parents, ARRAY_SIZE(sys_parents), 0,
>>
>
^ permalink raw reply
* [PATCH] fpga zynq: Check the bitstream for validity
From: Jason Gunthorpe @ 2016-11-08 16:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2e0ed43b-9a3e-48d4-4e8a-60d6f25ca4ee@suse.com>
On Tue, Nov 08, 2016 at 10:59:43AM +0100, Matthias Brugger wrote:
> > /* don't globally reset PL if we're doing partial reconfig */
> > if (!(flags & FPGA_MGR_PARTIAL_RECONFIG)) {
> >+ if (!zynq_fpga_has_sync(buf, count)) {
>
> Maybe we should add an error message here to let the user know what went
> wrong, as I think this error path could easily be hit by an user.
Sure, happy with this wording:
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index de475a6a1882..2f3da4c6e2e6 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -181,7 +181,7 @@ static irqreturn_t zynq_fpga_isr(int irq, void *data)
*/
static bool zynq_fpga_has_sync(const char *buf, size_t count)
{
- for (; count > 4; buf += 4, --count)
+ for (; count > 4; buf += 4, count -= 4)
if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
buf[3] == 0xaa)
return true;
@@ -200,8 +200,11 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
/* The hardware can only DMA multiples of 4 bytes, and we need at
* least the sync word and something else to do anything.
*/
- if (count <= 4 || (count % 4) != 0)
+ if (count <= 4 || (count % 4) != 0) {
+ dev_err(priv->dev,
+ "Invalid bitstream size, must be multiples of 4 bytes");
return -EINVAL;
+ }
err = clk_enable(priv->clk);
if (err)
@@ -210,6 +213,8 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
/* don't globally reset PL if we're doing partial reconfig */
if (!(flags & FPGA_MGR_PARTIAL_RECONFIG)) {
if (!zynq_fpga_has_sync(buf, count)) {
+ dev_err(priv->dev,
+ "Invalid bitstream, could not find a sync word. Bitstream must be a byte swaped .bin file");
err = -EINVAL;
goto out_err;
}
^ permalink raw reply related
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-08 16:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478576829-112707-4-git-send-email-yuanzhichang@hisilicon.com>
On Tuesday, November 8, 2016 11:47:09 AM CET zhichang.yuan wrote:
> + /*
> + * The first PCIBIOS_MIN_IO is reserved specifically for indirectIO.
> + * It will separate indirectIO range from pci host bridge to
> + * avoid the possible PIO conflict.
> + * Set the indirectIO range directly here.
> + */
> + lpcdev->io_ops.start = 0;
> + lpcdev->io_ops.end = PCIBIOS_MIN_IO - 1;
> + lpcdev->io_ops.devpara = lpcdev;
> + lpcdev->io_ops.pfin = hisilpc_comm_in;
> + lpcdev->io_ops.pfout = hisilpc_comm_out;
> + lpcdev->io_ops.pfins = hisilpc_comm_ins;
> + lpcdev->io_ops.pfouts = hisilpc_comm_outs;
I have to look at patch 2 in more detail again, after missing a few review
rounds. I'm still a bit skeptical about hardcoding a logical I/O port
range here, and would hope that we can just go through the same
assignment of logical port ranges that we have for PCI buses, decoupling
the bus addresses from the linux-internal ones.
Arnd
^ permalink raw reply
* [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: Arnd Bergmann @ 2016-11-08 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108114953.GB15297@leverpostej>
On Tuesday, November 8, 2016 11:49:53 AM CET Mark Rutland wrote:
> On Tue, Nov 08, 2016 at 11:47:08AM +0800, zhichang.yuan wrote:
> > +Hisilicon Hip06 low-pin-count device
> > + Usually LPC controller is part of PCI host bridge, so the legacy ISA ports
> > + locate on LPC bus can be accessed direclty. But some SoCs have independent
> > + LPC controller, and access the legacy ports by triggering LPC I/O cycles.
> > + Hisilicon Hip06 implements this LPC device.
>
> s/direclty/directly/
>
> My understanding of ISA (which may be flawed) is that it's not part of
> the PCI host bridge, but rather on x86 it happens to share the IO space
> with PCI.
On normal systems, ISA or LPC are behind a PCI bridge device, which
passes down both low addresses of I/O space and memory space.
> So, how about this becomes:
>
> Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which
> provides access to some legacy ISA devices.
>
> I believe that we could theoretically have multiple independent LPC/ISA
> busses, as is possible with PCI on !x86 systems. If the current ISA code
> assumes a singleton bus, I think that's something that needs to be fixed
> up more generically.
>
> I don't see why we should need any architecture-specific code here. Why
> can we not fix up the ISA bus code in drivers/of/address.c such that it
> handles multiple ISA bus instances, and translates all sub-device
> addresses relative to the specific bus instance?
I think it is a relatively safe assumption that there is only one
ISA bridge. A lot of old drivers hardcode PIO or memory addresses
when talking to an ISA device, so having multiple instances is
already problematic.
What is odd about ARM64 here is that the PIO space is not shared among
all ISA and PCI buses in some cases.
Arnd
^ permalink raw reply
* [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards
From: Gabriel Fernandez @ 2016-11-08 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFvLkMSwwyoi8Vb_-+6dViB8PaBRUoCZYBcrzDjgX9EJnv_mvQ@mail.gmail.com>
On 11/08/2016 09:52 AM, Rados?aw Pietrzyk wrote:
> 2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <gabriel.fernandez@st.com>:
>> Hi Rados?aw
>>
>> Many thanks for reviewing.
>>
>> On 11/07/2016 03:57 PM, Rados?aw Pietrzyk wrote:
>>>> +static struct clk_hw *clk_register_pll_div(const char *name,
>>>> + const char *parent_name, unsigned long flags,
>>>> + void __iomem *reg, u8 shift, u8 width,
>>>> + u8 clk_divider_flags, const struct clk_div_table *table,
>>>> + struct clk_hw *pll_hw, spinlock_t *lock)
>>>> +{
>>>> + struct stm32f4_pll_div *pll_div;
>>>> + struct clk_hw *hw;
>>>> + struct clk_init_data init;
>>>> + int ret;
>>>> +
>>>> + /* allocate the divider */
>>>> + pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>>>> + if (!pll_div)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + init.name = name;
>>>> + init.ops = &stm32f4_pll_div_ops;
>>>> + init.flags = flags;
>>> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
>>> should have CLK_SET_RATE_GATE flag and we can get rid of custom
>>> divider ops.
>> I don't want to offer the possibility to change the vco clock through the
>> divisor of the pll (only by a boot-loader or by DT).
>>
>> e.g. if i make a set rate on lcd-tft clock, i don't want to change the SAI
>> frequencies.
>>
>> I used same structure for internal divisors of the pll (p, q, r) and for
>> post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
>> That why the CLK_SET_RATE_PARENT flag is transmit by parameter.
>>
>> These divisors are similar because we have to switch off the pll before
>> changing the rate.
>>
> But changing pll and lcd dividers only may not be enough for getting
> very specific pixelclocks and that might require changing the VCO
> frequency itself. The rest of the SAI tree should be recalculated
> then.
I agree but it seems to be too much complicated to recalculate all PLL
divisors if we change the vco clock.
You mean to use Clock notifier callback ?
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Arnd Bergmann @ 2016-11-08 16:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <13493313.OkuDZEY5WO@wuerfel>
On Tuesday, November 8, 2016 5:09:59 PM CET Arnd Bergmann wrote:
>
> I don't see a better alternative. I earlier suggested having these
> out of line so we don't grow the object code too much when it is
> enabled.
>
On second look, I see that they are all done out of line, I would
just move around the BUILD_EXTIO macro to the file that uses it
and remove and open-code the DECLARE_EXTIO() as that makes it easier
to grep.
Arnd
^ permalink raw reply
* [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Russell King - ARM Linux @ 2016-11-08 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e83e8462-6d63-7253-f69c-afb507463bba@arm.com>
On Tue, Nov 08, 2016 at 04:08:38PM +0000, Sudeep Holla wrote:
> I will focus on this and see what's happening. I have issue with network
> on my setup with debug build and Linaro guys are not seeing it. I revert
> to release build of UEFI for that. Anyways one suggestion I have right
> now is to erase the second partition of NOR flash where the old UEFI
> config is placed and it could be conflicting with the new UEFI image.
>
> Cmd> flash
>
>
> Switching on main power...
> PMIC RAM configuration (pms_v105.bin)...
> IOFPGA config: PASSED
> Enabling usb remote...
>
>
> Flash> ERASERANGE 0x0BFC0000 0x0BFF0000
>
> I will looking into that further.
Still behaves the same, stopping at:
FV2 Hob 0xFE07B000 - 0xFE8253BF
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Will Deacon @ 2016-11-08 16:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478576829-112707-2-git-send-email-yuanzhichang@hisilicon.com>
On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> For arm64, there is no I/O space as other architectural platforms, such as
> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> known port addresses are used to control the corresponding target devices, for
> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> normal MMIO mode in using.
>
> To drive these devices, this patch introduces a method named indirect-IO.
> In this method the in/out pair in arch/arm64/include/asm/io.h will be
> redefined. When upper layer drivers call in/out with those known legacy port
> addresses to access the peripherals, the hooking functions corrresponding to
> those target peripherals will be called. Through this way, those upper layer
> drivers which depend on in/out can run on Hip06 without any changes.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> ---
> arch/arm64/Kconfig | 6 +++
> arch/arm64/include/asm/extio.h | 94 ++++++++++++++++++++++++++++++++++++++++++
> arch/arm64/include/asm/io.h | 29 +++++++++++++
> arch/arm64/kernel/Makefile | 1 +
> arch/arm64/kernel/extio.c | 27 ++++++++++++
> 5 files changed, 157 insertions(+)
> create mode 100644 arch/arm64/include/asm/extio.h
> create mode 100644 arch/arm64/kernel/extio.c
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 969ef88..b44070b 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -163,6 +163,12 @@ config ARCH_MMAP_RND_COMPAT_BITS_MIN
> config ARCH_MMAP_RND_COMPAT_BITS_MAX
> default 16
>
> +config ARM64_INDIRECT_PIO
> + bool "access peripherals with legacy I/O port"
> + help
> + Support special accessors for ISA I/O devices. This is needed for
> + SoCs that do not support standard read/write for the ISA range.
> +
> config NO_IOPORT_MAP
> def_bool y if !PCI
>
> diff --git a/arch/arm64/include/asm/extio.h b/arch/arm64/include/asm/extio.h
> new file mode 100644
> index 0000000..6ae0787
> --- /dev/null
> +++ b/arch/arm64/include/asm/extio.h
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __LINUX_EXTIO_H
> +#define __LINUX_EXTIO_H
> +
> +struct extio_ops {
> + unsigned long start;/* inclusive, sys io addr */
> + unsigned long end;/* inclusive, sys io addr */
> +
> + u64 (*pfin)(void *devobj, unsigned long ptaddr, size_t dlen);
> + void (*pfout)(void *devobj, unsigned long ptaddr, u32 outval,
> + size_t dlen);
> + u64 (*pfins)(void *devobj, unsigned long ptaddr, void *inbuf,
> + size_t dlen, unsigned int count);
> + void (*pfouts)(void *devobj, unsigned long ptaddr,
> + const void *outbuf, size_t dlen,
> + unsigned int count);
> + void *devpara;
> +};
> +
> +extern struct extio_ops *arm64_extio_ops;
> +
> +#define DECLARE_EXTIO(bw, type) \
> +extern type in##bw(unsigned long addr); \
> +extern void out##bw(type value, unsigned long addr); \
> +extern void ins##bw(unsigned long addr, void *buffer, unsigned int count);\
> +extern void outs##bw(unsigned long addr, const void *buffer, unsigned int count);
> +
> +#define BUILD_EXTIO(bw, type) \
> +type in##bw(unsigned long addr) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + return read##bw(PCI_IOBASE + addr); \
> + return arm64_extio_ops->pfin ? \
> + arm64_extio_ops->pfin(arm64_extio_ops->devpara, \
> + addr, sizeof(type)) : -1; \
> +} \
> + \
> +void out##bw(type value, unsigned long addr) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + write##bw(value, PCI_IOBASE + addr); \
> + else \
> + if (arm64_extio_ops->pfout) \
> + arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> + addr, value, sizeof(type)); \
> +} \
> + \
> +void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + reads##bw(PCI_IOBASE + addr, buffer, count); \
> + else \
> + if (arm64_extio_ops->pfins) \
> + arm64_extio_ops->pfins(arm64_extio_ops->devpara,\
> + addr, buffer, sizeof(type), count); \
> +} \
> + \
> +void outs##bw(unsigned long addr, const void *buffer, unsigned int count) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + writes##bw(PCI_IOBASE + addr, buffer, count); \
> + else \
> + if (arm64_extio_ops->pfouts) \
> + arm64_extio_ops->pfouts(arm64_extio_ops->devpara,\
> + addr, buffer, sizeof(type), count); \
> +}
> +
> +static inline void arm64_set_extops(struct extio_ops *ops)
> +{
> + if (ops)
> + WRITE_ONCE(arm64_extio_ops, ops);
Why does this need to be WRITE_ONCE? You don't have READ_ONCE on the reader
side. Also, what if multiple drivers want to set different ops for distinct
address ranges?
> +}
> +
> +#endif /* __LINUX_EXTIO_H*/
> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> index 0bba427..136735d 100644
> --- a/arch/arm64/include/asm/io.h
> +++ b/arch/arm64/include/asm/io.h
> @@ -31,6 +31,7 @@
> #include <asm/early_ioremap.h>
> #include <asm/alternative.h>
> #include <asm/cpufeature.h>
> +#include <asm/extio.h>
>
> #include <xen/xen.h>
>
> @@ -149,6 +150,34 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>
> +
> +/*
> + * redefine the in(s)b/out(s)b for indirect-IO.
> + */
> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> +#define inb inb
> +#define outb outb
> +#define insb insb
> +#define outsb outsb
> +/* external declaration */
> +DECLARE_EXTIO(b, u8)
> +
> +#define inw inw
> +#define outw outw
> +#define insw insw
> +#define outsw outsw
> +
> +DECLARE_EXTIO(w, u16)
> +
> +#define inl inl
> +#define outl outl
> +#define insl insl
> +#define outsl outsl
> +
> +DECLARE_EXTIO(l, u32)
> +#endif
> +
> +
> /*
> * String version of I/O memory access operations.
> */
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 7d66bba..60e0482 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -31,6 +31,7 @@ arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \
> sys_compat.o entry32.o
> arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o
> arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o
> +arm64-obj-$(CONFIG_ARM64_INDIRECT_PIO) += extio.o
> arm64-obj-$(CONFIG_ARM64_MODULE_PLTS) += module-plts.o
> arm64-obj-$(CONFIG_PERF_EVENTS) += perf_regs.o perf_callchain.o
> arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o
> diff --git a/arch/arm64/kernel/extio.c b/arch/arm64/kernel/extio.c
> new file mode 100644
> index 0000000..647b3fa
> --- /dev/null
> +++ b/arch/arm64/kernel/extio.c
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/io.h>
> +
> +struct extio_ops *arm64_extio_ops;
> +
> +
> +BUILD_EXTIO(b, u8)
> +
> +BUILD_EXTIO(w, u16)
> +
> +BUILD_EXTIO(l, u32)
Is there no way to make this slightly more generic, so that it can be
re-used elsewhere? For example, if struct extio_ops was common, then
you could have the singleton (which maybe should be an interval tree?),
type definition, setter function and the BUILD_EXTIO invocations
somewhere generic, rather than squirelled away in the arch backend.
Will
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Arnd Bergmann @ 2016-11-08 16:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108120323.GC15297@leverpostej>
On Tuesday, November 8, 2016 12:03:23 PM CET Mark Rutland wrote:
> On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> > For arm64, there is no I/O space as other architectural platforms, such as
> > X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> > such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> > known port addresses are used to control the corresponding target devices, for
> > example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> > normal MMIO mode in using.
>
> This has nothing to do with arm64. Hardware with this kind of indirect
> bus access could be integrated with a variety of CPU architectures. It
> simply hasn't been, yet.
Actually PowerPC has a vaguely similar mechanism.
> > To drive these devices, this patch introduces a method named indirect-IO.
> > In this method the in/out pair in arch/arm64/include/asm/io.h will be
> > redefined. When upper layer drivers call in/out with those known legacy port
> > addresses to access the peripherals, the hooking functions corrresponding to
> > those target peripherals will be called. Through this way, those upper layer
> > drivers which depend on in/out can run on Hip06 without any changes.
>
> As above, this has nothing to do with arm64, and as such, should live in
> generic code, exactly as we would do if we had higher-level ISA
> accessor ops.
>
> Regardless, given the multi-instance case, I don't think this is
> sufficient in general (and I think we need higher-level ISA accessors
> to handle the indirection).
I think it is rather unlikely that we have to deal with multiple
instances in the future, it's more likely that future platforms
won't have any I/O ports at all, which is why I was advocating for
simplicity here.
> > +type in##bw(unsigned long addr) \
> > +{ \
> > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > + arm64_extio_ops->end < addr) \
> > + return read##bw(PCI_IOBASE + addr); \
> > + return arm64_extio_ops->pfin ? \
> > + arm64_extio_ops->pfin(arm64_extio_ops->devpara, \
> > + addr, sizeof(type)) : -1; \
> > +} \
> > + \
> > +void out##bw(type value, unsigned long addr) \
> > +{ \
> > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > + arm64_extio_ops->end < addr) \
> > + write##bw(value, PCI_IOBASE + addr); \
> > + else \
> > + if (arm64_extio_ops->pfout) \
> > + arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > + addr, value, sizeof(type)); \
> > +} \
> > + \
> > +void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
> > +{ \
> > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > + arm64_extio_ops->end < addr) \
> > + reads##bw(PCI_IOBASE + addr, buffer, count); \
> > + else \
> > + if (arm64_extio_ops->pfins) \
> > + arm64_extio_ops->pfins(arm64_extio_ops->devpara,\
> > + addr, buffer, sizeof(type), count); \
> > +} \
> > + \
> > +void outs##bw(unsigned long addr, const void *buffer, unsigned int count) \
> > +{ \
> > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > + arm64_extio_ops->end < addr) \
> > + writes##bw(PCI_IOBASE + addr, buffer, count); \
> > + else \
> > + if (arm64_extio_ops->pfouts) \
> > + arm64_extio_ops->pfouts(arm64_extio_ops->devpara,\
> > + addr, buffer, sizeof(type), count); \
> > +}
> > +
>
> So all PCI I/O will be slowed down by irrelevant checks when this is
> enabled?
I don't see a better alternative. I earlier suggested having these
out of line so we don't grow the object code too much when it is
enabled.
Performance of PIO accessors is not an issue here though, any bus
access will by definition be orders of magnitude slower than the
added branches and dereferences here.
> [...]
>
> > +static inline void arm64_set_extops(struct extio_ops *ops)
> > +{
> > + if (ops)
> > + WRITE_ONCE(arm64_extio_ops, ops);
> > +}
>
> Why WRITE_ONCE()?
>
> Is this not protected/propagated by some synchronisation mechanism?
>
> WRITE_ONCE() is not sufficient to ensure that this is consistently
> observed by readers, and regardless, I don't see READ_ONCE() anywhere in
> this patch.
>
> This looks very suspicious.
Agreed, this looks wrong.
Arnd
^ 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