* [PATCH v2 1/3] arm64: dts: exynos: Add missing unit name to Exynos7 SoC node
From: Krzysztof Kozlowski @ 2017-01-10 18:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484069912-6534-2-git-send-email-javier@osg.samsung.com>
On Tue, Jan 10, 2017 at 02:38:30PM -0300, Javier Martinez Canillas wrote:
> This patch fixes the following DTC warning about a mismatch
> between a device node reg property and its unit name:
>
> Node /soc has a reg or ranges property, but no unit name
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
>
> arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> index 80aa60e38237..0d2fedc6ac2f 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> @@ -69,7 +69,7 @@
> method = "smc";
> };
>
> - soc: soc {
> + soc: soc at 0 {
This looks unnatural, like a fix just to silence the DTC. Mostly de do
not enumerate soc node, although there are few exceptions.
I would prefer ignore the warning... however I am happy to hear other opinions.
Best regards,
Krzysztof
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> --
> 2.7.4
>
^ permalink raw reply
* [RFC 7/8] KVM: arm/arm64: Set up a background timer for the physical timer emulation
From: Jintack Lim @ 2017-01-10 18:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170109121326.GE4348@cbox>
Hi Christoffer,
thanks for the review!
On Mon, Jan 9, 2017 at 7:13 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Mon, Dec 26, 2016 at 12:12:05PM -0500, Jintack Lim wrote:
>> Set a background timer for the EL1 physical timer emulation while VMs are
>> running, so that VMs get interrupts for the physical timer in a timely
>> manner.
>>
>> We still use just one background timer. When a VM is runnable, we use
>> the background timer for the physical timer emulation. When the VM is
>> about to be blocked, we use the background timer to wake up the vcpu at
>> the earliest timer expiration among timers the VM is using.
>>
>> As a result, the assumption that the background timer is not armed while
>> VMs are running does not hold any more. So, remove BUG_ON()s and
>> WARN_ON()s accordingly.
>>
>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> ---
>> virt/kvm/arm/arch_timer.c | 42 +++++++++++++++++++++++++++++++-----------
>> 1 file changed, 31 insertions(+), 11 deletions(-)
>>
>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
>> index aa7e243..be8d953 100644
>> --- a/virt/kvm/arm/arch_timer.c
>> +++ b/virt/kvm/arm/arch_timer.c
>> @@ -91,9 +91,6 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
>> vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired);
>> vcpu->arch.timer_cpu.armed = false;
>>
>> - WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)) &&
>> - !kvm_timer_should_fire(vcpu, vcpu_ptimer(vcpu)));
>> -
>
> This seems misplaced and has been addressed here:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2017-January/022933.html
>
> When you respin you can benefit from basing on that (assuming it gets
> acked and goes int).
Ok, I got it.
>
>> /*
>> * If the vcpu is blocked we want to wake it up so that it will see
>> * the timer has expired when entering the guest.
>> @@ -139,7 +136,6 @@ static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
>>
>> /*
>> * Returns minimal timer expiration time in ns among guest timers.
>> - * Note that it will return inf time if none of timers can fire.
>> */
>> static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
>> {
>> @@ -153,7 +149,9 @@ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
>> if (kvm_timer_irq_can_fire(ptimer))
>> min_phys = kvm_timer_compute_delta(vcpu, ptimer);
>>
>> - WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));
>> + /* If none of timers can fire, then return 0 */
>> + if ((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX))
>> + return 0;
>
> Why didn't you have this semantics in the previous patch?
I should have put this in the previous patch, and I'll do that.
Just let you know, WARN_ON() in the previous patch was there because I
thought that the caller of this function is sure that one of the
timers are able to fire. But I think that's beyond the scope of this
function.
>
>>
>> return min(min_virt, min_phys);
>> }
>> @@ -257,6 +255,26 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>> }
>>
>> /*
>> + * Schedule the background timer for the emulated timer. The background timer
>> + * runs whenever vcpu is runnable and the timer is not expired.
>> + */
>> +static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
>> + struct arch_timer_context *timer_ctx)
>> +{
>> + struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>> +
>> + if (kvm_timer_should_fire(vcpu, timer_ctx))
>> + return;
>> +
>> + if (!kvm_timer_irq_can_fire(timer_ctx))
>> + return;
>> +
>> + /* The timer has not yet expired, schedule a background timer */
>> + timer_disarm(timer);
>> + timer_arm(timer, kvm_timer_compute_delta(vcpu, timer_ctx));
>
> I'm wondering what the effect of this thing really is. Isn't the soft
> timer programmed in timer_arm() based on Linux's own timekeeping
> schedule, such that the physical timer will be programmed to the next
> tick, regardless of what you program here, so all you have to do is
> check if you need to inject the phys timer on entry to the VM?
>
> On the other hand, if this can cause Linux to program the phys timer to
> expire sooner, then I guess it makes sense. Thinking about it, would
> that be the case on a tickless system?
I don't have a good answer for this, so I'll get back to you!
>
>> +}
>> +
>> +/*
>> * Schedule the background timer before calling kvm_vcpu_block, so that this
>> * thread is removed from its waitqueue and made runnable when there's a timer
>> * interrupt to handle.
>> @@ -267,8 +285,6 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
>> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>> struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>>
>> - BUG_ON(timer_is_armed(timer));
>> -
>> /*
>> * No need to schedule a background timer if any guest timer has
>> * already expired, because kvm_vcpu_block will return before putting
>> @@ -290,13 +306,21 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
>> * The guest timers have not yet expired, schedule a background timer.
>> * Pick smaller expiration time between phys and virt timer.
>> */
>> + timer_disarm(timer);
>> timer_arm(timer, kvm_timer_min_block(vcpu));
>> }
>>
>> void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
>> {
>> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>> +
>> timer_disarm(timer);
>> +
>> + /*
>> + * Now we return from the blocking. If we have any timer to emulate,
>> + * and it's not expired, set the background timer for it.
>> + */
>> + kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
>
> hmm, this is only called when returning from the kvm_vcpu_block() path.
> What about when you do vcpu_load/put, don't you need to schedule/cancel
> it there too?
We can do that, but I think that's not necessary. Firing the physical
timer while a vcpu is unloaded doesn't affect the task scheduling. Or
is it awkward to do so?
>
> Maybe it's simpler to just program the soft timer during flush_hwstate
> and cancel the timer during sync_hwstate. Does that work?
As far as I remember, it worked. I agree that it's simpler.
But as I mentioned in the patch [8/8] reply this *may* cause more overhead.
>
>> }
>>
>> /**
>> @@ -375,10 +399,6 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
>> */
>> void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
>> {
>> - struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>> -
>> - BUG_ON(timer_is_armed(timer));
>> -
>> /*
>> * The guest could have modified the timer registers or the timer
>> * could have expired, update the timer state.
>> --
>> 1.9.1
>>
>>
>
> Thanks,
> -Christoffer
>
^ permalink raw reply
* [PATCH v2 3/3] arm64: dts: exynos: Remove unneeded unit names in Exynos5433 nodes
From: Krzysztof Kozlowski @ 2017-01-10 18:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484069912-6534-4-git-send-email-javier@osg.samsung.com>
On Tue, Jan 10, 2017 at 02:38:32PM -0300, Javier Martinez Canillas wrote:
> The "samsung,exynos5433-mipi-video-phy" and "samsung,exynos5250-dwusb3"
> DT bindings don't specify a reg property for these nodes, so having a
> unit name leads to the following DTC warnings:
>
> Node /soc/video-phy at 105c0710 has a unit name, but no reg property
> Node /soc/usb at 15400000 has a unit name, but no reg property
> Node /soc/usb at 15a00000 has a unit name, but no reg property
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> ---
>
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 3695ddaf2e04..17e5dafd392c 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -706,7 +706,7 @@
> interrupts = <GIC_PPI 9 0xf04>;
> };
>
> - mipi_phy: video-phy at 105c0710 {
> + mipi_phy: video-phy {
> compatible = "samsung,exynos5433-mipi-video-phy";
> #phy-cells = <1>;
> samsung,pmu-syscon = <&pmu_system_controller>;
> @@ -1285,7 +1285,7 @@
> status = "disabled";
> };
>
> - usbdrd30: usb at 15400000 {
> + usbdrd30: usb-0 {
How about "usbdrd" instead of "usb-0"? It would be still quite a generic
description of a class.
> compatible = "samsung,exynos5250-dwusb3";
> clocks = <&cmu_fsys CLK_ACLK_USBDRD30>,
> <&cmu_fsys CLK_SCLK_USBDRD30>;
> @@ -1332,7 +1332,7 @@
> status = "disabled";
> };
>
> - usbhost30: usb at 15a00000 {
> + usbhost30: usb-1 {
usbhost?
Best regards,
Krzysztof
> compatible = "samsung,exynos5250-dwusb3";
> clocks = <&cmu_fsys CLK_ACLK_USBHOST30>,
> <&cmu_fsys CLK_SCLK_USBHOST30>;
> --
> 2.7.4
>
^ permalink raw reply
* System/uncore PMUs and unit aggregation
From: Will Deacon @ 2017-01-10 18:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2cb0eb12-979c-7eff-7c51-ce9e06b3740c@codeaurora.org>
Hi Neil, Anurup, Jan,
On Thu, Nov 17, 2016 at 10:16:46PM -0500, Leeder, Neil wrote:
> Thanks for opening up the discussion on this Will.
>
> For the Qualcomm L2 driver, one objection I had to exposing each unit is
> that there are so many of them - the minimum starting point is a dozen, so
> trying to start 9 counters on each means a perf command line specifying 100+
> events. Future chips are only likely to increase that.
>
> There is a single CPU node so from an end-user perspective it would seems to
> make sense to also have a single L2 node. perf already has the ability to
> create events on multiple units using cpumask, aggregate the results, and
> split them out per unit with perf stat -a -A, so the user can get that
> granularity. Exposing separate units would make userspace duplicate a lot of
> that functionality. This may rely on each uncore unit being associated with
> a CPU, which is the case with L2.
>
> I agree with your comments regarding groups and I can see that a standard
> way of representing topology could be useful - in this case, which CPUs are
> within the same L2 cluster. Perhaps a list of cpumasks, one per L2 unit.
Mark and I had a chat about this earlier today and I think we largely agree
with you. That is, for composite PMUs with a notion of CPU affinity for
their component units, it makes sense to use the event affinity as a means
to address these units, rather than e.g. create separate PMU instances.
However, for PMUs that don't have this notion of affinity, the units should
either be exposed individually or, in the case that there is something like
shared control logic, they should be addressed through the config fields
(e.g. the hisilicon cache with the bank=NN option).
I think this fits with your driver, so please post an updated version
addressing Mark's unrelated review comments.
> On 11/17/2016 1:17 PM, Will Deacon wrote:
> [...]
> > 3. Summing the counters across units is only permitted if the units
> > can all be started and stopped atomically. Otherwise, the counters
> > should be exposed individually. It's up to the driver author to
> > decide what makes sense to sum.
>
> If I understand your your point 3 correctly, I'm not sure about the need to
> start and stop them all atomically. That seems to be a tighter requirement
> than we require for CPU PMUs. For them, perf stat -a creates events/groups
> on each CPU, then starts and stops them sequentially and sums the results.
> If that model is acceptable for the CPU to collect and aggregate counts,
> that should be the same bar that uncore PMUs need to reach. In the L2 case,
> the driver isn't summing the results, it's still perf doing it, so I may be
> misinterpreting your comment about where the summation is permitted.
My concern with summation is more that I don't want to expose high-level
"meta" events from the driver which in fact end up being a bunch of
different events in a bunch of different counters that can't be read
atomically. Userspace is free to do that, but the driver shouldn't claim
that it can support the event, if you see what I mean?
Will
^ permalink raw reply
* System/uncore PMUs and unit aggregation
From: Will Deacon @ 2017-01-10 18:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582EB89B.5050708@gmail.com>
On Fri, Nov 18, 2016 at 01:45:23PM +0530, Anurup M wrote:
> On Thursday 17 November 2016 11:47 PM, Will Deacon wrote:
> >We currently have support for three arm64 system PMUs in flight:
> >
> > [Cavium ThunderX] http://lkml.kernel.org/r/cover.1477741719.git.jglauber at cavium.com
> > [Hisilicon Hip0x] http://lkml.kernel.org/r/1478151727-20250-1-git-send-email-anurup.m at huawei.com
> > [Qualcomm L2] http://lkml.kernel.org/r/1477687813-11412-1-git-send-email-nleeder at codeaurora.org
> >
> >Each of which have to deal with multiple underlying hardware units in one
> >way or another. Mark and I recently expressed a desire to expose these
> >units to userspace as individual PMU instances, since this can allow:
> >
> > * Fine-grained control of events from userspace, when you want to see
> > individual numbers as opposed to a summed total
> >
> > * Potentially ease migration to new SoC revisions, where the units
> > are laid out slightly differently
> >
> > * Easier handling of cases where the units aren't quite identical
> >
> >however, this received pushback from all of the patch authors, so there's
> >clearly a problem with this approach. I'm hoping we can try to resolve
> >this here.
> >
> >Speaking to Mark earlier today, we came up with the following rough rules
> >for drivers that present multiple hardware units as a single PMU:
> >
> > 1. If the units share some part of the programming interface (e.g. control
> > registers or interrupts), then they must be handled by the same PMU.
> > Otherwise, they should be treated independently as separate PMU
> > instances.
> The Hisilicon Hip0x chip has units like L3 cache, Miscellaneous nodes, DDR
> controller etc.
> There are such units in multiple CPU die's in the chip.
>
> The L3 cache is further divided as banks which have separate set of
> interface (control registers, interrupts etc..).
> As per the suggestion, each L3 cache banks will be exposed as a individual
> PMU instance.
> So for e.g. in a board using Hip0x chip with 2 sockets and each socket
> consists of 2 CPU die,
> There will be a total of 16 L3 cache PMU's which will be exposed.
>
> My doubt here is
> Each L3 cache PMU has total 22 statistics events. So if registered as a
> separate PMU, will it not
> create multiple entries (with same event names) in event listing for
> multiple L3 cache PMU's.
> Is there a way to avoid this? or this is acceptable?
>
> Just a thought, If we can group them as single PMU and add a config
> parameter in the event listing to
> identify the L3 cache bank(sub unit). e.g: event name will appear as
> "hisi_l3c2/read_allocate,bank=?/".
> And user can choose count from bank 0x01 as -e
> "hisi_l3c2/read_allocate,bank=0x01/".
> And for aggregate count, bank=0xff.
> Does it over complicate? Please share your comments.
Adding a bank field to the config looks fine to me. I'm assuming the banks
aren't CPU affine?
Will
^ permalink raw reply
* [PATCH] ARM: dts: udoo_neo: Add UDOO Neo USB OTG1 and OTG2 support
From: Breno Lima @ 2017-01-10 18:59 UTC (permalink / raw)
To: linux-arm-kernel
Add support for micro USB (OTG1) and USB Host (OTG2) for UDOO Neo board.
Tested on a UDOO Neo Full board by mounting a mass storage device
on both ports.
Signed-off-by: Breno Lima <breno.lima@nxp.com>
---
arch/arm/boot/dts/imx6sx-udoo-neo.dtsi | 60 ++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sx-udoo-neo.dtsi b/arch/arm/boot/dts/imx6sx-udoo-neo.dtsi
index 2b65d26..827ace1 100644
--- a/arch/arm/boot/dts/imx6sx-udoo-neo.dtsi
+++ b/arch/arm/boot/dts/imx6sx-udoo-neo.dtsi
@@ -75,6 +75,28 @@
regulator-max-microvolt = <3300000>;
regulator-boot-on;
};
+
+ reg_usb_otg1_vbus: usb-otg1-vbus {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_otg1_reg>;
+ regulator-name = "usb_otg1_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&gpio1 9 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ reg_usb_otg2_vbus: usb-otg2-vbus {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_otg2_reg>;
+ regulator-name = "usb_otg2_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&gpio4 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
};
&cpu0 {
@@ -241,6 +263,28 @@
<MX6SX_PAD_CSI_DATA07__UART6_CTS_B 0x1b0b1>;
};
+ pinctrl_otg1_reg: otg1grp {
+ fsl,pins =
+ <MX6SX_PAD_GPIO1_IO09__GPIO1_IO_9 0x10b0>;
+ };
+
+
+ pinctrl_otg2_reg: otg2grp {
+ fsl,pins =
+ <MX6SX_PAD_NAND_RE_B__GPIO4_IO_12 0x10b0>;
+ };
+
+ pinctrl_usb_otg1: usbotg1grp {
+ fsl,pins =
+ <MX6SX_PAD_GPIO1_IO10__ANATOP_OTG1_ID 0x17059>,
+ <MX6SX_PAD_GPIO1_IO08__USB_OTG1_OC 0x10b0>;
+ };
+
+ pinctrl_usb_otg2: usbot2ggrp {
+ fsl,pins =
+ <MX6SX_PAD_QSPI1A_DATA0__USB_OTG2_OC 0x10b0>;
+ };
+
pinctrl_usdhc2: usdhc2grp {
fsl,pins =
<MX6SX_PAD_SD2_CMD__USDHC2_CMD 0x17059>,
@@ -280,6 +324,22 @@
status = "disabled";
};
+&usbotg1 { /* J2 micro USB port */
+ vbus-supply = <®_usb_otg1_vbus>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_otg1>;
+ imx6-usb-charger-detection;
+ status = "okay";
+};
+
+&usbotg2 { /* J3 host USB port */
+ vbus-supply = <®_usb_otg2_vbus>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_otg2>;
+ dr_mode = "host";
+ status = "okay";
+};
+
&usdhc2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usdhc2>;
--
2.7.4
^ permalink raw reply related
* [PATCH] usb: dwc3-exynos fix unspecified suspend clk error handling
From: Krzysztof Kozlowski @ 2017-01-10 18:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <23cf318d-b83f-55e6-e980-13133f0ceb8b@osg.samsung.com>
On Tue, Jan 10, 2017 at 11:37:24AM -0700, Shuah Khan wrote:
> On 01/10/2017 11:23 AM, Bartlomiej Zolnierkiewicz wrote:
> > I also think that regardless of what is decided on making susp_clk
> > non-optional for some Exynos SoCs we should probably remove the debug
> > message as it doesn't bring useful information and may be confusing.
> >
> > Shuah, can you take care of this?
>
> Yes. This message as it reads now is not only confusing, but also can
> lead users to think something is wrong.
>
> I can get rid of it or I could change it from info to debug and change
> it to read:
>
> "Optional Suspend clock isn't found. Diver operation isn't impacted"
It is even more confusing. If the clock is required (by binding, by
hardware) - make it an error. If it is completely not important - do not
print anything. If it is optional but helpful (enabling clock gives
someything) then print something... but it is not that case.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2 3/3] arm64: dts: exynos: Remove unneeded unit names in Exynos5433 nodes
From: Javier Martinez Canillas @ 2017-01-10 19:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110185109.n3x25yxbaarzikcd@kozik-lap>
Hello Krzysztof,
On 01/10/2017 03:51 PM, Krzysztof Kozlowski wrote:
[snip]
>>
>> - usbdrd30: usb at 15400000 {
>> + usbdrd30: usb-0 {
>
> How about "usbdrd" instead of "usb-0"? It would be still quite a generic
> description of a class.
>
>> compatible = "samsung,exynos5250-dwusb3";
>> clocks = <&cmu_fsys CLK_ACLK_USBDRD30>,
>> <&cmu_fsys CLK_SCLK_USBDRD30>;
>> @@ -1332,7 +1332,7 @@
>> status = "disabled";
>> };
>>
>> - usbhost30: usb at 15a00000 {
>> + usbhost30: usb-1 {
>
> usbhost?
>
Indeed, these sounds better so I'll change for them.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [linux-sunxi] Re: [PATCH 3/6] clk: sunxi-ng: Add H5 clocks
From: Maxime Ripard @ 2017-01-10 19:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170109161500.Ep9GCmIb@smtp2p.mail.yandex.net>
On Mon, Jan 09, 2017 at 09:13:12PM +0800, Icenowy Zheng wrote:
>
> 2017?1?9? ??7:01? Maxime Ripard <maxime.ripard@free-electrons.com>???
> >
> > On Fri, Jan 06, 2017 at 06:48:31AM +0800, Icenowy Zheng wrote:
> > >
> > > 2017?1?6? 06:04? Maxime Ripard <maxime.ripard@free-electrons.com>???
> > > >
> > > > On Tue, Dec 27, 2016 at 12:25:15AM +0800, Icenowy Zheng wrote:
> > > > > Add the H5 CCU clocks set based on the H3 one.
> > > > >
> > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > >
> > > > Is there any difference with H3's?
> > >
> > > One more Transport Stream controller, so one more bus gate and bus
> > > reset for it.
> >
> > There's no need to duplicate more than 1000 lines of code just for
> > that then. Just add a new compatible and reuse the clocks already
> > defined.
>
> How can I do this? Add them in ccu-sun8i-h3.c ?
Yes, you can have a look at how I did it for sun5i for example if you
want.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170110/47a0649c/attachment.sig>
^ permalink raw reply
* [PATCH v2 6/6] arm64: allwinner: a64: Increase the MMC max frequency
From: Maxime Ripard @ 2017-01-10 19:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v67JoOAtHjMJboNUdCDX=+=Hf+zOWr234rT7GEiUmPb=nQ@mail.gmail.com>
Hi,
On Tue, Jan 10, 2017 at 01:01:20AM +0800, Chen-Yu Tsai wrote:
> On Tue, Jan 10, 2017 at 12:46 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > All the controllers can have a maximum frequency of 200MHz.
> >
> > Since older SoCs cannot go that high, we cannot change the default maximum
> > frequency, but fortunately for us we have a property for that in the DT.
> >
> > This also has the side effect of allowing to use the MMC HS200 mode for the
> > boards that support it (with either 1.2v or 1.8v IOs).
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
> > 1 file changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index 8e149498e096..f46ae965cf5b 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -332,6 +332,7 @@
> > resets = <&ccu RST_BUS_MMC0>;
> > reset-names = "ahb";
> > interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> > + max-frequency = <200000000>;
>
> You also have to set one of MMC_CAP2_HS200* in the driver,
> or mmc-hs200-1_8v or mmc-hs200-1_2v in the device tree to
> actually use HS200, right?
Yes, but that requires a board with 1.8V IOs to work properly, which
not all board use, so it's probably best to enable it in the board
DTS.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170110/de9c272e/attachment.sig>
^ permalink raw reply
* [PATCH] usb: dwc3-exynos fix unspecified suspend clk error handling
From: Shuah Khan @ 2017-01-10 19:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110185943.qxwnzjg6zzy4zbki@kozik-lap>
On 01/10/2017 11:59 AM, Krzysztof Kozlowski wrote:
> On Tue, Jan 10, 2017 at 11:37:24AM -0700, Shuah Khan wrote:
>> On 01/10/2017 11:23 AM, Bartlomiej Zolnierkiewicz wrote:
>>> I also think that regardless of what is decided on making susp_clk
>>> non-optional for some Exynos SoCs we should probably remove the debug
>>> message as it doesn't bring useful information and may be confusing.
>>>
>>> Shuah, can you take care of this?
>>
>> Yes. This message as it reads now is not only confusing, but also can
>> lead users to think something is wrong.
>>
>> I can get rid of it or I could change it from info to debug and change
>> it to read:
>>
>> "Optional Suspend clock isn't found. Diver operation isn't impacted"
>
> It is even more confusing. If the clock is required (by binding, by
> hardware) - make it an error. If it is completely not important - do not
> print anything. If it is optional but helpful (enabling clock gives
> someything) then print something... but it is not that case.
>
> Best regards,
> Krzysztof
>
Sounds fair. I will send a patch to remove the message.
-- Shuah
^ permalink raw reply
* [RFC 4/8] KVM: arm/arm64: Initialize the emulated EL1 physical timer
From: Christoffer Dall @ 2017-01-10 19:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHyh4xgq_me42Jv+=6P=NhcyxpjJ4R6zgWGY2HibEc6e09Af2g@mail.gmail.com>
On Tue, Jan 10, 2017 at 12:03:00PM -0500, Jintack Lim wrote:
> On Mon, Jan 9, 2017 at 7:02 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Mon, Dec 26, 2016 at 12:12:02PM -0500, Jintack Lim wrote:
> >> Initialize the emulated EL1 physical timer with the default irq number.
> >>
> >> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> >> ---
> >> arch/arm/kvm/reset.c | 9 ++++++++-
> >> arch/arm64/kvm/reset.c | 9 ++++++++-
> >> include/kvm/arm_arch_timer.h | 3 ++-
> >> virt/kvm/arm/arch_timer.c | 12 ++++++++++--
> >> 4 files changed, 28 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/arm/kvm/reset.c b/arch/arm/kvm/reset.c
> >> index 4b5e802..1da8b2d 100644
> >> --- a/arch/arm/kvm/reset.c
> >> +++ b/arch/arm/kvm/reset.c
> >> @@ -37,6 +37,11 @@
> >> .usr_regs.ARM_cpsr = SVC_MODE | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
> >> };
> >>
> >> +static const struct kvm_irq_level cortexa_ptimer_irq = {
> >> + { .irq = 30 },
> >> + .level = 1,
> >> +};
> >> +
> >> static const struct kvm_irq_level cortexa_vtimer_irq = {
> >> { .irq = 27 },
> >> .level = 1,
> >> @@ -58,6 +63,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >> {
> >> struct kvm_regs *reset_regs;
> >> const struct kvm_irq_level *cpu_vtimer_irq;
> >> + const struct kvm_irq_level *cpu_ptimer_irq;
> >>
> >> switch (vcpu->arch.target) {
> >> case KVM_ARM_TARGET_CORTEX_A7:
> >> @@ -65,6 +71,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >> reset_regs = &cortexa_regs_reset;
> >> vcpu->arch.midr = read_cpuid_id();
> >> cpu_vtimer_irq = &cortexa_vtimer_irq;
> >> + cpu_ptimer_irq = &cortexa_ptimer_irq;
> >> break;
> >> default:
> >> return -ENODEV;
> >> @@ -77,5 +84,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >> kvm_reset_coprocs(vcpu);
> >>
> >> /* Reset arch_timer context */
> >> - return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> >> + return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
> >> }
> >> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> >> index 5bc4608..74322c2 100644
> >> --- a/arch/arm64/kvm/reset.c
> >> +++ b/arch/arm64/kvm/reset.c
> >> @@ -46,6 +46,11 @@
> >> COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT),
> >> };
> >>
> >> +static const struct kvm_irq_level default_ptimer_irq = {
> >> + .irq = 30,
> >> + .level = 1,
> >> +};
> >> +
> >> static const struct kvm_irq_level default_vtimer_irq = {
> >> .irq = 27,
> >> .level = 1,
> >> @@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
> >> int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >> {
> >> const struct kvm_irq_level *cpu_vtimer_irq;
> >> + const struct kvm_irq_level *cpu_ptimer_irq;
> >> const struct kvm_regs *cpu_reset;
> >>
> >> switch (vcpu->arch.target) {
> >> @@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >> }
> >>
> >> cpu_vtimer_irq = &default_vtimer_irq;
> >> + cpu_ptimer_irq = &default_ptimer_irq;
> >> break;
> >> }
> >>
> >> @@ -136,5 +143,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >> kvm_pmu_vcpu_reset(vcpu);
> >>
> >> /* Reset timer */
> >> - return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> >> + return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
> >> }
> >> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> >> index d21652a..04ed9c1 100644
> >> --- a/include/kvm/arm_arch_timer.h
> >> +++ b/include/kvm/arm_arch_timer.h
> >> @@ -61,7 +61,8 @@ struct arch_timer_cpu {
> >> int kvm_timer_enable(struct kvm_vcpu *vcpu);
> >> void kvm_timer_init(struct kvm *kvm);
> >> int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> >> - const struct kvm_irq_level *irq);
> >> + const struct kvm_irq_level *virt_irq,
> >> + const struct kvm_irq_level *phys_irq);
> >> void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu);
> >> void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu);
> >> void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu);
> >> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> >> index 3bd6063..ed80864 100644
> >> --- a/virt/kvm/arm/arch_timer.c
> >> +++ b/virt/kvm/arm/arch_timer.c
> >> @@ -339,9 +339,11 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
> >> }
> >>
> >> int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> >> - const struct kvm_irq_level *irq)
> >> + const struct kvm_irq_level *virt_irq,
> >> + const struct kvm_irq_level *phys_irq)
> >> {
> >> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> >> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> >>
> >> /*
> >> * The vcpu timer irq number cannot be determined in
> >> @@ -349,7 +351,8 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> >> * kvm_vcpu_set_target(). To handle this, we determine
> >> * vcpu timer irq number when the vcpu is reset.
> >> */
> >> - vtimer->irq.irq = irq->irq;
> >> + vtimer->irq.irq = virt_irq->irq;
> >> + ptimer->irq.irq = phys_irq->irq;
> >>
> >> /*
> >> * The bits in CNTV_CTL are architecturally reset to UNKNOWN for ARMv8
> >> @@ -358,6 +361,7 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> >> * the ARMv7 architecture.
> >> */
> >> vtimer->cnt_ctl = 0;
> >> + ptimer->cnt_ctl = 0;
> >> kvm_timer_update_state(vcpu);
> >>
> >> return 0;
> >> @@ -477,11 +481,15 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
> >> int kvm_timer_enable(struct kvm_vcpu *vcpu)
> >> {
> >> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> >> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> >> struct irq_desc *desc;
> >> struct irq_data *data;
> >> int phys_irq;
> >> int ret;
> >>
> >> + /* Always enable emulated the EL1 physical timer */
> >
> > Dubious comment the way it stands.
> >
> > Does the rest of the code really support one timer being enabled while
> > another one not so?
>
> No. The code never check if the physical timer is enabled. I think
> it's not necessary to set enable bit for this emulated physical timer.
> We, however, may need to set/check this enable bit later if we decide
> to give the EL1 physical timer to the guest instead of emulating it.
>
It hink the semantics of the enable bit before your patches is more "is
the arch timer code at all enabled and working", so maybe you want to
preserve that and move the enabled bit out of the per-timer and per-vcpu
state?
Thanks,
-Christoffer
^ permalink raw reply
* [RFC 7/8] KVM: arm/arm64: Set up a background timer for the physical timer emulation
From: Christoffer Dall @ 2017-01-10 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHyh4xhvjR2k5H4kTM3t_rXyzih-h2aHQTN9VPqrzJbJ=Zuq0g@mail.gmail.com>
On Tue, Jan 10, 2017 at 01:47:49PM -0500, Jintack Lim wrote:
> Hi Christoffer,
>
> thanks for the review!
>
> On Mon, Jan 9, 2017 at 7:13 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Mon, Dec 26, 2016 at 12:12:05PM -0500, Jintack Lim wrote:
> >> Set a background timer for the EL1 physical timer emulation while VMs are
> >> running, so that VMs get interrupts for the physical timer in a timely
> >> manner.
> >>
> >> We still use just one background timer. When a VM is runnable, we use
> >> the background timer for the physical timer emulation. When the VM is
> >> about to be blocked, we use the background timer to wake up the vcpu at
> >> the earliest timer expiration among timers the VM is using.
> >>
> >> As a result, the assumption that the background timer is not armed while
> >> VMs are running does not hold any more. So, remove BUG_ON()s and
> >> WARN_ON()s accordingly.
> >>
> >> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> >> ---
> >> virt/kvm/arm/arch_timer.c | 42 +++++++++++++++++++++++++++++++-----------
> >> 1 file changed, 31 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> >> index aa7e243..be8d953 100644
> >> --- a/virt/kvm/arm/arch_timer.c
> >> +++ b/virt/kvm/arm/arch_timer.c
> >> @@ -91,9 +91,6 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
> >> vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired);
> >> vcpu->arch.timer_cpu.armed = false;
> >>
> >> - WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)) &&
> >> - !kvm_timer_should_fire(vcpu, vcpu_ptimer(vcpu)));
> >> -
> >
> > This seems misplaced and has been addressed here:
> > https://lists.cs.columbia.edu/pipermail/kvmarm/2017-January/022933.html
> >
> > When you respin you can benefit from basing on that (assuming it gets
> > acked and goes int).
>
> Ok, I got it.
>
> >
> >> /*
> >> * If the vcpu is blocked we want to wake it up so that it will see
> >> * the timer has expired when entering the guest.
> >> @@ -139,7 +136,6 @@ static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
> >>
> >> /*
> >> * Returns minimal timer expiration time in ns among guest timers.
> >> - * Note that it will return inf time if none of timers can fire.
> >> */
> >> static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
> >> {
> >> @@ -153,7 +149,9 @@ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
> >> if (kvm_timer_irq_can_fire(ptimer))
> >> min_phys = kvm_timer_compute_delta(vcpu, ptimer);
> >>
> >> - WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));
> >> + /* If none of timers can fire, then return 0 */
> >> + if ((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX))
> >> + return 0;
> >
> > Why didn't you have this semantics in the previous patch?
>
> I should have put this in the previous patch, and I'll do that.
>
> Just let you know, WARN_ON() in the previous patch was there because I
> thought that the caller of this function is sure that one of the
> timers are able to fire. But I think that's beyond the scope of this
> function.
>
> >
> >>
> >> return min(min_virt, min_phys);
> >> }
> >> @@ -257,6 +255,26 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
> >> }
> >>
> >> /*
> >> + * Schedule the background timer for the emulated timer. The background timer
> >> + * runs whenever vcpu is runnable and the timer is not expired.
> >> + */
> >> +static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
> >> + struct arch_timer_context *timer_ctx)
> >> +{
> >> + struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> >> +
> >> + if (kvm_timer_should_fire(vcpu, timer_ctx))
> >> + return;
> >> +
> >> + if (!kvm_timer_irq_can_fire(timer_ctx))
> >> + return;
> >> +
> >> + /* The timer has not yet expired, schedule a background timer */
> >> + timer_disarm(timer);
> >> + timer_arm(timer, kvm_timer_compute_delta(vcpu, timer_ctx));
> >
> > I'm wondering what the effect of this thing really is. Isn't the soft
> > timer programmed in timer_arm() based on Linux's own timekeeping
> > schedule, such that the physical timer will be programmed to the next
> > tick, regardless of what you program here, so all you have to do is
> > check if you need to inject the phys timer on entry to the VM?
> >
> > On the other hand, if this can cause Linux to program the phys timer to
> > expire sooner, then I guess it makes sense. Thinking about it, would
> > that be the case on a tickless system?
>
> I don't have a good answer for this, so I'll get back to you!
>
> >
> >> +}
> >> +
> >> +/*
> >> * Schedule the background timer before calling kvm_vcpu_block, so that this
> >> * thread is removed from its waitqueue and made runnable when there's a timer
> >> * interrupt to handle.
> >> @@ -267,8 +285,6 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
> >> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> >> struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> >>
> >> - BUG_ON(timer_is_armed(timer));
> >> -
> >> /*
> >> * No need to schedule a background timer if any guest timer has
> >> * already expired, because kvm_vcpu_block will return before putting
> >> @@ -290,13 +306,21 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
> >> * The guest timers have not yet expired, schedule a background timer.
> >> * Pick smaller expiration time between phys and virt timer.
> >> */
> >> + timer_disarm(timer);
> >> timer_arm(timer, kvm_timer_min_block(vcpu));
> >> }
> >>
> >> void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
> >> {
> >> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> >> +
> >> timer_disarm(timer);
> >> +
> >> + /*
> >> + * Now we return from the blocking. If we have any timer to emulate,
> >> + * and it's not expired, set the background timer for it.
> >> + */
> >> + kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
> >
> > hmm, this is only called when returning from the kvm_vcpu_block() path.
> > What about when you do vcpu_load/put, don't you need to schedule/cancel
> > it there too?
>
> We can do that, but I think that's not necessary. Firing the physical
> timer while a vcpu is unloaded doesn't affect the task scheduling. Or
> is it awkward to do so?
>
Not sure I understand your question.
> >
> > Maybe it's simpler to just program the soft timer during flush_hwstate
> > and cancel the timer during sync_hwstate. Does that work?
>
> As far as I remember, it worked. I agree that it's simpler.
> But as I mentioned in the patch [8/8] reply this *may* cause more overhead.
>
I think you should go for something we can easily convince ourselves
will work initially, and then we can measure if there's significant
overhead and improve on that.
At least we should have a clear idea of the when to schedule software
timers and why.
We know from my RFC series to optimize the timer behavior that on some
systems, not having to read/write the timer registers on every put/load
can save us a few hundred cycles, so it's not extremely expensive, but
we should at the same time avoid it if possible.
Assuming most guests will use the virt timer for now, if we can just
have an initial check on both flush/sync if the ptimer is even enabled,
and only do extra work when that's the case, then I think that would be
a good start.
Thanks,
-Christoffer
^ permalink raw reply
* [RFC 8/8] KVM: arm/arm64: Emulate the EL1 phys timer register access
From: Christoffer Dall @ 2017-01-10 19:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHyh4xhNUTuzaf=JJKidHj1fnOgoe_8aTZamnnGsc6C+iKL1BA@mail.gmail.com>
On Tue, Jan 10, 2017 at 12:36:36PM -0500, Jintack Lim wrote:
> On Mon, Jan 9, 2017 at 7:16 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
> >> Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
> >> Now the VM is able to use the EL1 physical timer.
> >>
> >> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> >> ---
> >> arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
> >> include/kvm/arm_arch_timer.h | 3 +++
> >> virt/kvm/arm/arch_timer.c | 4 ++--
> >> 3 files changed, 37 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> >> index fd9e747..7cef94f 100644
> >> --- a/arch/arm64/kvm/sys_regs.c
> >> +++ b/arch/arm64/kvm/sys_regs.c
> >> @@ -824,7 +824,15 @@ static bool access_cntp_tval(struct kvm_vcpu *vcpu,
> >> struct sys_reg_params *p,
> >> const struct sys_reg_desc *r)
> >> {
> >> - kvm_inject_undefined(vcpu);
> >> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> >> + cycle_t now = kvm_phys_timer_read();
> >> +
> >> + if (p->is_write) {
> >> + ptimer->cnt_cval = p->regval + now;
> >> + kvm_timer_emulate(vcpu, ptimer);
> >
> > Hmm, do we really need those calls here?
> >
> > I guess I'm a little confused about exactly what the kvm_timer_emulate()
> > function is supposed to do, and it feels to me like these handlers
> > should just record what the guest is asking the kernel to do and the
> > logic of handling the additional timer should be moved into the run path
> > as much as possible.
>
> I think it's a design decision. As you suggested, it's simple to do
> set up the background timer on entry to the VM, cancel it on exit, but
> since that's on the critical path it may have some impact on the
> performance, especially the world switch cost. To avoid
> canceling/setting up timer every world switch, I choose to schedule
> the physical timer here. I haven't compared the cost of the two
> alternatives, though.
>
I'd definitely like to avoid us scheduling soft timers on the host if
that's not even necessary in the first place, so I'd like to get that
clear first, and as I said on the previous patch I think it's better to
get a working solution that we understand firt, and then optimize on
that later based on real results.
-Christoffer
^ permalink raw reply
* [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
From: Heiko Stübner @ 2017-01-10 19:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=Wwu3q_LqwYUWcJQRvp5neVOS9szgsYFONWTRJ0X8hRTA@mail.gmail.com>
Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
> Hi,
>
> On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com>
wrote:
> > The structure rockchip_clk_provider needs to refer the GRF regmap
> > in somewhere, if the CRU node has not "rockchip,grf" property,
> > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> > regmap, and the MUXGRF type clock will be not supported.
> >
> > Therefore, we need to add them.
> >
> > Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> > ---
> >
> > Changes in v4:
> > - separte the binding patch
> >
> > Changes in v3:
> > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
> >
> > Changes in v2:
> > - referring pmugrf for PMUGRU
> > - fix the typo "invaild" in COMMIT message
> >
> > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> > 1 file changed, 2 insertions(+)
>
> This seems fine to me, so:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> ...but I will say that before you actually add any real "MUXGRF"
> clocks on rk3399 you _might_ need to rework the code to make things
> truly "optional". If it turns out that any existing clocks that
> already exist today already go through one of these muxes in the GRF
> and we've always been assuming one setting of the mux, we'll need to
> make sure we keep assuming that setting of the mux even if the "grf"
> isn't specified.
I guess I see that a bit more relaxed :-) .
I.e. the GRF being optional is a remnant of syscons not being available when
the clocks get set up- so were coming in later or not at all. For the rk3288 I
converted, there we never really had the case of the GRF missing.
And the GRF mux for the vcodec now present is not being used by anything yet
(neither driver nor binding), so no old devicetree can break.
> As I understand it, your motivation for this patch is to eventually be
> able to model the EDP reference clock which can either be xin24 or
> "edp osc". Presumably the eDP "reference clock" isn't related to any
> of the pre-existing eDP clocks so that one should be safe.
Same here, so far we don't even have edp or even any other graphical output on
the rk3399, so again there is no old devicetree that could break when the grf
is not specified.
So, for me that looks good.
Heiko
^ permalink raw reply
* [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables
From: Nate Watterson @ 2017-01-10 19:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161220102209.GC10132@arm.com>
In the current arm-smmu-v3 driver, all smmus that support 2-level
stream tables are being forced to use them. This is suboptimal for
smmus that support fewer stream id bits than would fill in a single
second level table. This patch limits the use of 2-level tables to
smmus that both support the feature and whose first level table can
possibly contain more than a single entry.
Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
---
drivers/iommu/arm-smmu-v3.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d6ec44..7d1a7e5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1983,17 +1983,9 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
u32 size, l1size;
struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
- /*
- * If we can resolve everything with a single L2 table, then we
- * just need a single L1 descriptor. Otherwise, calculate the L1
- * size, capped to the SIDSIZE.
- */
- if (smmu->sid_bits < STRTAB_SPLIT) {
- size = 0;
- } else {
- size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
- size = min(size, smmu->sid_bits - STRTAB_SPLIT);
- }
+ /* Calculate the L1 size, capped to the SIDSIZE. */
+ size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
+ size = min(size, smmu->sid_bits - STRTAB_SPLIT);
cfg->num_l1_ents = 1 << size;
size += STRTAB_SPLIT;
@@ -2504,6 +2496,13 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
smmu->ssid_bits = reg >> IDR1_SSID_SHIFT & IDR1_SSID_MASK;
smmu->sid_bits = reg >> IDR1_SID_SHIFT & IDR1_SID_MASK;
+ /*
+ * If the SMMU supports fewer bits than would fill a single L2 stream
+ * table, use a linear table instead.
+ */
+ if (smmu->sid_bits <= STRTAB_SPLIT)
+ smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB;
+
/* IDR5 */
reg = readl_relaxed(smmu->base + ARM_SMMU_IDR5);
--
Qualcomm Datacenter Technologies, Inc. on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH] of: alloc anywhere from memblock if range not specified
From: Laura Abbott @ 2017-01-10 19:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <717b117d-9b8b-d4e9-6135-3c52afd635c8@gmail.com>
On 01/10/2017 08:16 AM, Leonard Crestez wrote:
> Hello,
>
> I have some trouble with this patch.
>
> It seems the intention is to allow CMA to be placed in highmem. If the CMA area is
> larger than highmem and no alloc-ranges is specified (just a size) it is possible
> to end up allocating a area that spans from multiple zones. This later breaks
> checks in cma_activate_area and makes most dma allocations fail.
>
> Am I missing something or this a bug?
>
This has been discussed in previous threads
https://marc.info/?l=linux-kernel&m=147990760506179&w=2
https://marc.info/?l=linux-kernel&m=147928325113103&w=2
I haven't seen any follow up since then though.
> On Mon, Feb 22, 2016 at 3:45 PM, Vinayak Menon <vinmenon@codeaurora.org> wrote:
>>
>> early_init_dt_alloc_reserved_memory_arch passes end as 0 to
>> __memblock_alloc_base, when limits are not specified. But
>> __memblock_alloc_base takes end value of 0 as MEMBLOCK_ALLOC_ACCESSIBLE
>> and limits the end to memblock.current_limit. This results in regions
>> never being placed in HIGHMEM area, for e.g. CMA.
>> Let __memblock_alloc_base allocate from anywhere in memory if limits are
>> not specified.
>>
>> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>> ---
>> drivers/of/of_reserved_mem.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>> index 1a3556a..ed01c01 100644
>> --- a/drivers/of/of_reserved_mem.c
>> +++ b/drivers/of/of_reserved_mem.c
>> @@ -32,11 +32,13 @@ int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>> phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
>> phys_addr_t *res_base)
>> {
>> + phys_addr_t base;
>> /*
>> * We use __memblock_alloc_base() because memblock_alloc_base()
>> * panic()s on allocation failure.
>> */
>> - phys_addr_t base = __memblock_alloc_base(size, align, end);
>> + end = !end ? MEMBLOCK_ALLOC_ANYWHERE : end;
>> + base = __memblock_alloc_base(size, align, end);
>> if (!base)
>> return -ENOMEM;
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
>> member of the Code Aurora Forum, hosted by The Linux Foundation
>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 1/3] arm64: dts: exynos: Add missing unit name to Exynos7 SoC node
From: Javier Martinez Canillas @ 2017-01-10 19:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110184728.2nff4r634ics4wb3@kozik-lap>
Hello Krzysztof,
On 01/10/2017 03:47 PM, Krzysztof Kozlowski wrote:
> On Tue, Jan 10, 2017 at 02:38:30PM -0300, Javier Martinez Canillas wrote:
>> This patch fixes the following DTC warning about a mismatch
>> between a device node reg property and its unit name:
>>
>> Node /soc has a reg or ranges property, but no unit name
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> ---
>>
>> arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> index 80aa60e38237..0d2fedc6ac2f 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> @@ -69,7 +69,7 @@
>> method = "smc";
>> };
>>
>> - soc: soc {
>> + soc: soc at 0 {
>
> This looks unnatural, like a fix just to silence the DTC. Mostly de do
> not enumerate soc node, although there are few exceptions.
>
Yes, but OTOH arm32 Exynos SoCs just have an empty "ranges" property in their
soc device node (parent and child address space is the same, no translation)
so DTC doesn't complain about the unit address in those.
But others SoCs DTSI with a non-empty ranges property have an unit name in
their soc nodes, i.e for arm64 and arm32:
arch/arm64/boot/dts/marvell/berlin4ct.dtsi
arch/arm/boot/dts/da850.dtsi
> I would prefer ignore the warning... however I am happy to hear other opinions.
>
If is wrong/unnatural to have addresses for soc nodes then I think DTC should
be patched to ignore these (like it will be the case for the OPP nodes AFAIU).
> Best regards,
> Krzysztof
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
From: Heiko Stübner @ 2017-01-10 19:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8683375.pL9J7LxuT0@diego>
Hi Doug,
Am Dienstag, 10. Januar 2017, 20:46:12 schrieb Heiko St?bner:
> Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
> > Hi,
> >
> > On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com>
>
> wrote:
> > > The structure rockchip_clk_provider needs to refer the GRF regmap
> > > in somewhere, if the CRU node has not "rockchip,grf" property,
> > > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> > > regmap, and the MUXGRF type clock will be not supported.
> > >
> > > Therefore, we need to add them.
> > >
> > > Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> > > ---
> > >
> > > Changes in v4:
> > > - separte the binding patch
> > >
> > > Changes in v3:
> > > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
> > >
> > > Changes in v2:
> > > - referring pmugrf for PMUGRU
> > > - fix the typo "invaild" in COMMIT message
> > >
> > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> > > 1 file changed, 2 insertions(+)
> >
> > This seems fine to me, so:
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >
> > ...but I will say that before you actually add any real "MUXGRF"
> > clocks on rk3399 you _might_ need to rework the code to make things
> > truly "optional". If it turns out that any existing clocks that
> > already exist today already go through one of these muxes in the GRF
> > and we've always been assuming one setting of the mux, we'll need to
> > make sure we keep assuming that setting of the mux even if the "grf"
> > isn't specified.
>
> I guess I see that a bit more relaxed :-) .
>
> I.e. the GRF being optional is a remnant of syscons not being available when
> the clocks get set up- so were coming in later or not at all. For the
> rk3288 I converted, there we never really had the case of the GRF missing.
>
> And the GRF mux for the vcodec now present is not being used by anything yet
> (neither driver nor binding), so no old devicetree can break.
>
> > As I understand it, your motivation for this patch is to eventually be
> > able to model the EDP reference clock which can either be xin24 or
> > "edp osc". Presumably the eDP "reference clock" isn't related to any
> > of the pre-existing eDP clocks so that one should be safe.
>
> Same here, so far we don't even have edp or even any other graphical output
> on the rk3399, so again there is no old devicetree that could break when
> the grf is not specified.
reading all of the above again, it feels like you essentially also said
similar things already in your original reply and I misread some of it.
But again, I don't see the need for any more code right now, as hopefully the
simple stuff we currently only support does not have any grf-based muxes in
it. Xing + Rockchip people, please correct me if I'm wrong here :-)
Heiko
^ permalink raw reply
* [RFC 8/8] KVM: arm/arm64: Emulate the EL1 phys timer register access
From: Jintack Lim @ 2017-01-10 20:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110194040.GP4348@cbox>
On Tue, Jan 10, 2017 at 2:40 PM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Tue, Jan 10, 2017 at 12:36:36PM -0500, Jintack Lim wrote:
>> On Mon, Jan 9, 2017 at 7:16 AM, Christoffer Dall
>> <christoffer.dall@linaro.org> wrote:
>> > On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
>> >> Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
>> >> Now the VM is able to use the EL1 physical timer.
>> >>
>> >> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> >> ---
>> >> arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
>> >> include/kvm/arm_arch_timer.h | 3 +++
>> >> virt/kvm/arm/arch_timer.c | 4 ++--
>> >> 3 files changed, 37 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> >> index fd9e747..7cef94f 100644
>> >> --- a/arch/arm64/kvm/sys_regs.c
>> >> +++ b/arch/arm64/kvm/sys_regs.c
>> >> @@ -824,7 +824,15 @@ static bool access_cntp_tval(struct kvm_vcpu *vcpu,
>> >> struct sys_reg_params *p,
>> >> const struct sys_reg_desc *r)
>> >> {
>> >> - kvm_inject_undefined(vcpu);
>> >> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>> >> + cycle_t now = kvm_phys_timer_read();
>> >> +
>> >> + if (p->is_write) {
>> >> + ptimer->cnt_cval = p->regval + now;
>> >> + kvm_timer_emulate(vcpu, ptimer);
>> >
>> > Hmm, do we really need those calls here?
>> >
>> > I guess I'm a little confused about exactly what the kvm_timer_emulate()
>> > function is supposed to do, and it feels to me like these handlers
>> > should just record what the guest is asking the kernel to do and the
>> > logic of handling the additional timer should be moved into the run path
>> > as much as possible.
>>
>> I think it's a design decision. As you suggested, it's simple to do
>> set up the background timer on entry to the VM, cancel it on exit, but
>> since that's on the critical path it may have some impact on the
>> performance, especially the world switch cost. To avoid
>> canceling/setting up timer every world switch, I choose to schedule
>> the physical timer here. I haven't compared the cost of the two
>> alternatives, though.
>>
>
> I'd definitely like to avoid us scheduling soft timers on the host if
> that's not even necessary in the first place, so I'd like to get that
> clear first, and as I said on the previous patch I think it's better to
> get a working solution that we understand firt, and then optimize on
> that later based on real results.
Ok, it makes sense. I'll respin!
>
> -Christoffer
>
^ permalink raw reply
* [PATCH net-next 0/8] net: dsa: Support for pdata in dsa2
From: Florian Fainelli @ 2017-01-10 20:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
This is not exactly new, and was sent before, although back then, I did not
have an user of the pre-declared MDIO board information, but now we do. Note
that I have additional changes queued up to have b53 register platform data for
MIPS bcm47xx and bcm63xx.
Yes I know that we should have the Orion platforms eventually be converted to
Device Tree, but until that happens, I don't want any remaining users of the
old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
and, there will be platforms out there that most likely won't never see DT
coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
future).
We would probably want the whole series to be merged via David Miller's tree
to simplify things.
Thanks!
Changes from last submission (few months back):
- rebased against latest net-next
- do not introduce dsa2_platform_data which was overkill and was meant to
allow us to do exaclty the same things with platform data and Device Tree
we use the existing dsa_platform_data instead
- properly register MDIO devices when the MDIO bus is registered and associate
platform_data with them
- add a change to the Orion platform code to demonstrate how this can be used
Thank you
Florian Fainelli (8):
net: dsa: Pass device pointer to dsa_register_switch
net: dsa: Make most functions take a dsa_port argument
net: dsa: Suffix function manipulating device_node with _dn
net: dsa: Move ports assignment closer to error checking
net: dsa: Export dev_to_net_device()
net: dsa: Add support for platform data
net: phy: Allow pre-declaration of MDIO devices
ARM: orion: Register DSA switch as a MDIO device
arch/arm/plat-orion/common.c | 19 ++++-
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 8 +-
drivers/net/dsa/qca8k.c | 2 +-
drivers/net/phy/Makefile | 3 +-
drivers/net/phy/mdio-boardinfo.c | 86 ++++++++++++++++++++
drivers/net/phy/mdio-boardinfo.h | 19 +++++
drivers/net/phy/mdio_bus.c | 5 ++
drivers/net/phy/mdio_device.c | 11 +++
include/linux/mdio.h | 3 +
include/linux/mod_devicetable.h | 1 +
include/linux/phy.h | 19 +++++
include/net/dsa.h | 4 +-
net/dsa/dsa.c | 18 +++--
net/dsa/dsa2.c | 169 +++++++++++++++++++++++++++------------
net/dsa/dsa_priv.h | 4 +-
16 files changed, 301 insertions(+), 72 deletions(-)
create mode 100644 drivers/net/phy/mdio-boardinfo.c
create mode 100644 drivers/net/phy/mdio-boardinfo.h
--
2.9.3
^ permalink raw reply
* [PATCH net-next 1/8] net: dsa: Pass device pointer to dsa_register_switch
From: Florian Fainelli @ 2017-01-10 20:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110201235.21771-1-f.fainelli@gmail.com>
In preparation for allowing dsa_register_switch() to be supplied with
device/platform data, pass down a struct device pointer instead of a
struct device_node.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 11 ++++++-----
drivers/net/dsa/qca8k.c | 2 +-
include/net/dsa.h | 2 +-
net/dsa/dsa2.c | 7 ++++---
5 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 5102a3701a1a..7179eed9ee6d 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1882,7 +1882,7 @@ int b53_switch_register(struct b53_device *dev)
pr_info("found switch: %s, rev %i\n", dev->name, dev->core_rev);
- return dsa_register_switch(dev->ds, dev->ds->dev->of_node);
+ return dsa_register_switch(dev->ds, dev->ds->dev);
}
EXPORT_SYMBOL(b53_switch_register);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index eea8e0176e33..1060597e160a 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4407,8 +4407,7 @@ static struct dsa_switch_driver mv88e6xxx_switch_drv = {
.ops = &mv88e6xxx_switch_ops,
};
-static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
- struct device_node *np)
+static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
{
struct device *dev = chip->dev;
struct dsa_switch *ds;
@@ -4423,7 +4422,7 @@ static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
dev_set_drvdata(dev, ds);
- return dsa_register_switch(ds, np);
+ return dsa_register_switch(ds, dev);
}
static void mv88e6xxx_unregister_switch(struct mv88e6xxx_chip *chip)
@@ -4507,9 +4506,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (err)
goto out_g2_irq;
- err = mv88e6xxx_register_switch(chip, np);
- if (err)
+ err = mv88e6xxx_register_switch(chip);
+ if (err) {
+ mv88e6xxx_mdio_unregister(chip);
goto out_mdio;
+ }
return 0;
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 54d270d59eb0..c084aa484d2b 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -964,7 +964,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
mutex_init(&priv->reg_mutex);
dev_set_drvdata(&mdiodev->dev, priv);
- return dsa_register_switch(priv->ds, priv->ds->dev->of_node);
+ return dsa_register_switch(priv->ds, &mdiodev->dev);
}
static void
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b94d1f2ef912..16a502a6c26a 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -403,7 +403,7 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
}
void dsa_unregister_switch(struct dsa_switch *ds);
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev);
#ifdef CONFIG_PM_SLEEP
int dsa_switch_suspend(struct dsa_switch *ds);
int dsa_switch_resume(struct dsa_switch *ds);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index bad119cee2a3..f0e3212ae9d5 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -592,8 +592,9 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
return ports;
}
-static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
+ struct device_node *np = dev->of_node;
struct device_node *ports = dsa_get_ports(ds, np);
struct dsa_switch_tree *dst;
u32 tree, index;
@@ -673,12 +674,12 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
return err;
}
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
int err;
mutex_lock(&dsa2_mutex);
- err = _dsa_register_switch(ds, np);
+ err = _dsa_register_switch(ds, dev);
mutex_unlock(&dsa2_mutex);
return err;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 2/8] net: dsa: Make most functions take a dsa_port argument
From: Florian Fainelli @ 2017-01-10 20:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110201235.21771-1-f.fainelli@gmail.com>
In preparation for allowing platform data, and therefore no valid
device_node pointer, make most DSA functions takes a pointer to a
dsa_port structure whenever possible. While at it, introduce a
dsa_port_is_valid() helper function which checks whether port->dn is
NULL or not at the moment.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 15 ++++++++------
net/dsa/dsa2.c | 61 +++++++++++++++++++++++++++++-------------------------
net/dsa/dsa_priv.h | 4 ++--
3 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index fd532487dfdf..2306d1b87c83 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -110,8 +110,9 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
/* basic switch operations **************************************************/
int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct device_node *port_dn, int port)
+ struct dsa_port *dport, int port)
{
+ struct device_node *port_dn = dport->dn;
struct phy_device *phydev;
int ret, mode;
@@ -141,15 +142,15 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
{
- struct device_node *port_dn;
+ struct dsa_port *dport;
int ret, port;
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
- port_dn = ds->ports[port].dn;
- ret = dsa_cpu_dsa_setup(ds, dev, port_dn, port);
+ dport = &ds->ports[port];
+ ret = dsa_cpu_dsa_setup(ds, dev, dport, port);
if (ret)
return ret;
}
@@ -366,8 +367,10 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
return ds;
}
-void dsa_cpu_dsa_destroy(struct device_node *port_dn)
+void dsa_cpu_dsa_destroy(struct dsa_port *port)
{
+ struct device_node *port_dn = port->dn;
+
if (of_phy_is_fixed_link(port_dn))
of_phy_deregister_fixed_link(port_dn);
}
@@ -393,7 +396,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
- dsa_cpu_dsa_destroy(ds->ports[port].dn);
+ dsa_cpu_dsa_destroy(&ds->ports[port]);
/* Clearing a bit which is not set does no harm */
ds->cpu_port_mask |= ~(1 << port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index f0e3212ae9d5..91141ac6ec18 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -79,11 +79,16 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
kref_put(&dst->refcount, dsa_free_dst);
}
-static bool dsa_port_is_dsa(struct device_node *port)
+static bool dsa_port_is_valid(struct dsa_port *port)
+{
+ return !!port->dn;
+}
+
+static bool dsa_port_is_dsa(struct dsa_port *port)
{
const char *name;
- name = of_get_property(port, "label", NULL);
+ name = of_get_property(port->dn, "label", NULL);
if (!name)
return false;
@@ -93,11 +98,11 @@ static bool dsa_port_is_dsa(struct device_node *port)
return false;
}
-static bool dsa_port_is_cpu(struct device_node *port)
+static bool dsa_port_is_cpu(struct dsa_port *port)
{
const char *name;
- name = of_get_property(port, "label", NULL);
+ name = of_get_property(port->dn, "label", NULL);
if (!name)
return false;
@@ -138,7 +143,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
static int dsa_port_complete(struct dsa_switch_tree *dst,
struct dsa_switch *src_ds,
- struct device_node *port,
+ struct dsa_port *port,
u32 src_port)
{
struct device_node *link;
@@ -146,7 +151,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
struct dsa_switch *dst_ds;
for (index = 0;; index++) {
- link = of_parse_phandle(port, "link", index);
+ link = of_parse_phandle(port->dn, "link", index);
if (!link)
break;
@@ -169,13 +174,13 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
*/
static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (!dsa_port_is_dsa(port))
@@ -215,7 +220,7 @@ static int dsa_dst_complete(struct dsa_switch_tree *dst)
return 0;
}
-static int dsa_dsa_port_apply(struct device_node *port, u32 index,
+static int dsa_dsa_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
int err;
@@ -230,13 +235,13 @@ static int dsa_dsa_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_dsa_port_unapply(struct device_node *port, u32 index,
+static void dsa_dsa_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
dsa_cpu_dsa_destroy(port);
}
-static int dsa_cpu_port_apply(struct device_node *port, u32 index,
+static int dsa_cpu_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
int err;
@@ -253,7 +258,7 @@ static int dsa_cpu_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
+static void dsa_cpu_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
dsa_cpu_dsa_destroy(port);
@@ -261,13 +266,13 @@ static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
}
-static int dsa_user_port_apply(struct device_node *port, u32 index,
+static int dsa_user_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
const char *name;
int err;
- name = of_get_property(port, "label", NULL);
+ name = of_get_property(port->dn, "label", NULL);
err = dsa_slave_create(ds, ds->dev, index, name);
if (err) {
@@ -279,7 +284,7 @@ static int dsa_user_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_user_port_unapply(struct device_node *port, u32 index,
+static void dsa_user_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
if (ds->ports[index].netdev) {
@@ -291,7 +296,7 @@ static void dsa_user_port_unapply(struct device_node *port, u32 index,
static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
@@ -325,8 +330,8 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
}
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_dsa(port)) {
@@ -353,12 +358,12 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_dsa(port)) {
@@ -439,7 +444,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
dst->applied = false;
}
-static int dsa_cpu_parse(struct device_node *port, u32 index,
+static int dsa_cpu_parse(struct dsa_port *port, u32 index,
struct dsa_switch_tree *dst,
struct dsa_switch *ds)
{
@@ -447,7 +452,7 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
struct net_device *ethernet_dev;
struct device_node *ethernet;
- ethernet = of_parse_phandle(port, "ethernet", 0);
+ ethernet = of_parse_phandle(port->dn, "ethernet", 0);
if (!ethernet)
return -EINVAL;
@@ -480,13 +485,13 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_cpu(port)) {
@@ -547,7 +552,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
* to have access to a correct value, just like what
* net/dsa/dsa.c::dsa_switch_setup_one does.
*/
- if (!dsa_port_is_cpu(port))
+ if (!dsa_port_is_cpu(&ds->ports[reg]))
ds->enabled_port_mask |= 1 << reg;
}
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7e3385ec73f4..a015ec97c289 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -50,8 +50,8 @@ struct dsa_slave_priv {
/* dsa.c */
int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct device_node *port_dn, int port);
-void dsa_cpu_dsa_destroy(struct device_node *port_dn);
+ struct dsa_port *dport, int port);
+void dsa_cpu_dsa_destroy(struct dsa_port *dport);
const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds);
void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 3/8] net: dsa: Suffix function manipulating device_node with _dn
From: Florian Fainelli @ 2017-01-10 20:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110201235.21771-1-f.fainelli@gmail.com>
Make it clear that these functions take a device_node structure pointer
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 91141ac6ec18..9089b3b1d7f5 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -112,8 +112,8 @@ static bool dsa_port_is_cpu(struct dsa_port *port)
return false;
}
-static bool dsa_ds_find_port(struct dsa_switch *ds,
- struct device_node *port)
+static bool dsa_ds_find_port_dn(struct dsa_switch *ds,
+ struct device_node *port)
{
u32 index;
@@ -123,8 +123,8 @@ static bool dsa_ds_find_port(struct dsa_switch *ds,
return false;
}
-static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
- struct device_node *port)
+static struct dsa_switch *dsa_dst_find_port_dn(struct dsa_switch_tree *dst,
+ struct device_node *port)
{
struct dsa_switch *ds;
u32 index;
@@ -134,7 +134,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
if (!ds)
continue;
- if (dsa_ds_find_port(ds, port))
+ if (dsa_ds_find_port_dn(ds, port))
return ds;
}
@@ -155,7 +155,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
if (!link)
break;
- dst_ds = dsa_dst_find_port(dst, link);
+ dst_ds = dsa_dst_find_port_dn(dst, link);
of_node_put(link);
if (!dst_ds)
@@ -559,7 +559,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
return 0;
}
-static int dsa_parse_member(struct device_node *np, u32 *tree, u32 *index)
+static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
{
int err;
@@ -605,7 +605,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
u32 tree, index;
int i, err;
- err = dsa_parse_member(np, &tree, &index);
+ err = dsa_parse_member_dn(np, &tree, &index);
if (err)
return err;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 4/8] net: dsa: Move ports assignment closer to error checking
From: Florian Fainelli @ 2017-01-10 20:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110201235.21771-1-f.fainelli@gmail.com>
Move the assignment of ports in _dsa_register_switch() closer to where
it is checked, no functional change. Re-order declarations to be
preserve the inverted christmas tree style.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 9089b3b1d7f5..ddee540d9a83 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -600,8 +600,8 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
struct device_node *np = dev->of_node;
- struct device_node *ports = dsa_get_ports(ds, np);
struct dsa_switch_tree *dst;
+ struct device_node *ports;
u32 tree, index;
int i, err;
@@ -609,6 +609,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
if (err)
return err;
+ ports = dsa_get_ports(ds, np);
if (IS_ERR(ports))
return PTR_ERR(ports);
--
2.9.3
^ permalink raw reply related
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