* [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
From: Chanwoo Choi @ 2016-11-24 7:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <58368C91.8030502@rock-chips.com>
+ Tobias Jakobi,
Hi Lin,
We need to discuss how to support the suspend-opp of devfreq device.
Now, there are two patch thread for suspend-opp of devfreq.
The Lin's approach modify the devfreq_suspend_device() to support suspend-opp.
The Tobias's approach[1] add new devfreq_suspend() and then call it on dpm_suspend()
when entering the suspend state.
[1] [RFC 0/4] PM / devfreq: draft for OPP suspend impl
- https://patchwork.kernel.org/patch/9443323/
- https://patchwork.kernel.org/patch/9443325/
- https://patchwork.kernel.org/patch/9443329/
- https://patchwork.kernel.org/patch/9443331/
I think we need to discuss it together.
Regards,
Chanwoo Choi
On 2016? 11? 24? 15:45, hl wrote:
> Hi MyungJoo Ham,
>
> On 2016?11?24? 14:14, MyungJoo Ham wrote:
>> On Thu, Nov 24, 2016 at 11:18 AM, hl <hl@rock-chips.com> wrote:
>>> Hi MyungJoo Ham,
>> []
>>>> We still need to sync the all status even i call target() in
>>>> devfreq_suspend/resume_device
>>>> directly, so still need update_devfreq() other setp except
>>>> devfreq->governor->get_target_freq(devfreq, &freq);
>>> And i think it better to be governor behaviors, for userspace they may not
>>> want to change
>>> the suspend frequency like other governor, the frequency should decide by
>>> the user, if they
>>> want this function, they should like other governor to rigister a
>>> devfreq_monitor_suspend().
>>
>>> What do you think about my rev6 patch?
>> If I understand the intention correctly, this is for the stability of
>> the device due to the behavior or bootloader/SoC-initializer, which
>> has nothing to do with governors.
>>
>> Even if users are using userspace, as long as they set the custom
>> frequencies lower than the default, they have the possibility of
>> being unstable as ondemand is going to have.
>>
>>
>> To reuse the update_devfreq() code, you may do something like:
>>
>> static int _update_freq(struct devfreq *devfreq, bool is_suspending)
>> {
>> /* original contents of update_freq with if statement with is_suspending wrapping get_target_freq */
>> }
>> int update_freq(struct devfreq *devfreq)
>> {
>> return _update_freq(devfreq, false);
>> }
>>
>>
>> There should be other good non-invasive methods that are not governoe-specific as well.
>>
> Thanks for your suggestion, i will update the new version soon.
>>
>> Cheers,
>> MyungJoo
>>
>>
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
> --
> Lin Huang
>
^ permalink raw reply
* [PATCH][v2] arm64: Add DTS support for FSL's LS1012A SoC
From: Yao Yuan @ 2016-11-24 7:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479320647-24460-1-git-send-email-harninder.rai@nxp.com>
Hi Shawn and All,
Any comment for this LS1012A platform support patch?
Is this a good enough base for consequence dts update patches?
If yes, I'd like to send some dts patches for DSPI and QSPI based on this patch.
Thanks.
Yao
On 11/17/2016 02:24 AM, Harninder Rai wrote:
> LS1012A features an advanced 64-bit ARM v8 CortexA53 processor with 32 KB
> of parity protected L1-I cache, 32 KB of ECC protected L1-D cache, as well as 256
> KB of ECC protected L2 cache.
>
> Features summary
> One 64-bit ARM-v8 Cortex-A53 core with the following capabilities
> - Arranged as a cluster of one core supporting a 256 KB L2 cache with ECC
> protection
> - Speed up to 800 MHz
> - Parity-protected 32 KB L1 instruction cache and 32 KB L1 data cache
> - Neon SIMD engine
> - ARM v8 cryptography extensions
> One 16-bit DDR3L SDRAM memory controller ARM core-link CCI-400 cache
> coherent interconnect Cryptography acceleration (SEC) One Configurable x3
> SerDes One PCI Express Gen2 controller, supporting x1 operation One serial
> ATA (SATA Gen 3.0) controller One USB 3.0/2.0 controller with integrated PHY
>
> Following levels of DTSI/DTS files have been created for the LS1012A
> SoC family:
>
> - fsl-ls1012a.dtsi:
> DTS-Include file for FSL LS1012A SoC.
>
> - fsl-ls1012a-frdm.dts:
> DTS file for FSL LS1012A FRDM board.
>
> - fsl-ls1012a-qds.dts:
> DTS file for FSL LS1012A QDS board.
>
> - fsl-ls1012a-rdb.dts:
> DTS file for FSL LS1012A RDB board.
>
> Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
> Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
> ---
> Changes in v2: Incorporated Shawn's comments
> - Brief introduction of the SoC in commit message
> - Alphabetic ordering of labeled nodes
> - Better naming to be used for regulator node
> - Make timer node's comments more readable
> - Sort nodes with unit-address in order of the address
>
> arch/arm64/boot/dts/freescale/Makefile | 3 +
> arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts | 115 ++++++++++
> arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts | 128 +++++++++++
> arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts | 59 +++++
> arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 245
> +++++++++++++++++++++
> 5 files changed, 550 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
>
^ permalink raw reply
* [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
From: hl @ 2016-11-24 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5836927B.9010205@samsung.com>
Hi Chanwoo Choi,
I think the dev_pm_opp_get_suspend_opp() have implement most of
the funtion, all we need is just define the node in dts, like following:
&dmc_opp_table {
opp06 {
opp-suspend;
};
};
so i think my way semm more simple.
On 2016?11?24? 15:10, Chanwoo Choi wrote:
> + Tobias Jakobi,
>
> Hi Lin,
>
> We need to discuss how to support the suspend-opp of devfreq device.
> Now, there are two patch thread for suspend-opp of devfreq.
>
> The Lin's approach modify the devfreq_suspend_device() to support suspend-opp.
> The Tobias's approach[1] add new devfreq_suspend() and then call it on dpm_suspend()
> when entering the suspend state.
>
> [1] [RFC 0/4] PM / devfreq: draft for OPP suspend impl
> - https://patchwork.kernel.org/patch/9443323/
> - https://patchwork.kernel.org/patch/9443325/
> - https://patchwork.kernel.org/patch/9443329/
> - https://patchwork.kernel.org/patch/9443331/
>
> I think we need to discuss it together.
>
> Regards,
> Chanwoo Choi
>
> On 2016? 11? 24? 15:45, hl wrote:
>> Hi MyungJoo Ham,
>>
>> On 2016?11?24? 14:14, MyungJoo Ham wrote:
>>> On Thu, Nov 24, 2016 at 11:18 AM, hl <hl@rock-chips.com> wrote:
>>>> Hi MyungJoo Ham,
>>> []
>>>>> We still need to sync the all status even i call target() in
>>>>> devfreq_suspend/resume_device
>>>>> directly, so still need update_devfreq() other setp except
>>>>> devfreq->governor->get_target_freq(devfreq, &freq);
>>>> And i think it better to be governor behaviors, for userspace they may not
>>>> want to change
>>>> the suspend frequency like other governor, the frequency should decide by
>>>> the user, if they
>>>> want this function, they should like other governor to rigister a
>>>> devfreq_monitor_suspend().
>>>> What do you think about my rev6 patch?
>>> If I understand the intention correctly, this is for the stability of
>>> the device due to the behavior or bootloader/SoC-initializer, which
>>> has nothing to do with governors.
>>>
>>> Even if users are using userspace, as long as they set the custom
>>> frequencies lower than the default, they have the possibility of
>>> being unstable as ondemand is going to have.
>>>
>>>
>>> To reuse the update_devfreq() code, you may do something like:
>>>
>>> static int _update_freq(struct devfreq *devfreq, bool is_suspending)
>>> {
>>> /* original contents of update_freq with if statement with is_suspending wrapping get_target_freq */
>>> }
>>> int update_freq(struct devfreq *devfreq)
>>> {
>>> return _update_freq(devfreq, false);
>>> }
>>>
>>>
>>> There should be other good non-invasive methods that are not governoe-specific as well.
>>>
>> Thanks for your suggestion, i will update the new version soon.
>>> Cheers,
>>> MyungJoo
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Linux-rockchip mailing list
>>> Linux-rockchip at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>> --
>> Lin Huang
>>
>
>
>
--
Lin Huang
^ permalink raw reply
* [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
From: Chanwoo Choi @ 2016-11-24 8:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5836980F.6050006@rock-chips.com>
Hi Lin,
On 2016? 11? 24? 16:34, hl wrote:
> Hi Chanwoo Choi,
>
> I think the dev_pm_opp_get_suspend_opp() have implement most of
> the funtion, all we need is just define the node in dts, like following:
>
> &dmc_opp_table {
> opp06 {
> opp-suspend;
> };
> };
Two approaches use the 'opp-suspend' property.
I think that the method to support suspend-opp have to
guarantee following conditions:
- Support the all of devfreq's governors.
- Devfreq framework have the responsibility to change the
frequency/voltage for suspend-opp. If we uses the
new devfreq_suspend(), each devfreq device don't care
how to support the suspend-opp. Just the developer of each
devfreq device need to add 'opp-suspend' propet to OPP entry in DT file.
Best Regards,
Chanwoo Choi
>
> so i think my way semm more simple.
>
> On 2016?11?24? 15:10, Chanwoo Choi wrote:
>> + Tobias Jakobi,
>>
>> Hi Lin,
>>
>> We need to discuss how to support the suspend-opp of devfreq device.
>> Now, there are two patch thread for suspend-opp of devfreq.
>>
>> The Lin's approach modify the devfreq_suspend_device() to support suspend-opp.
>> The Tobias's approach[1] add new devfreq_suspend() and then call it on dpm_suspend()
>> when entering the suspend state.
>>
>> [1] [RFC 0/4] PM / devfreq: draft for OPP suspend impl
>> - https://patchwork.kernel.org/patch/9443323/
>> - https://patchwork.kernel.org/patch/9443325/
>> - https://patchwork.kernel.org/patch/9443329/
>> - https://patchwork.kernel.org/patch/9443331/
>>
>> I think we need to discuss it together.
>>
>> Regards,
>> Chanwoo Choi
>>
>> On 2016? 11? 24? 15:45, hl wrote:
>>> Hi MyungJoo Ham,
>>>
>>> On 2016?11?24? 14:14, MyungJoo Ham wrote:
>>>> On Thu, Nov 24, 2016 at 11:18 AM, hl <hl@rock-chips.com> wrote:
>>>>> Hi MyungJoo Ham,
>>>> []
>>>>>> We still need to sync the all status even i call target() in
>>>>>> devfreq_suspend/resume_device
>>>>>> directly, so still need update_devfreq() other setp except
>>>>>> devfreq->governor->get_target_freq(devfreq, &freq);
>>>>> And i think it better to be governor behaviors, for userspace they may not
>>>>> want to change
>>>>> the suspend frequency like other governor, the frequency should decide by
>>>>> the user, if they
>>>>> want this function, they should like other governor to rigister a
>>>>> devfreq_monitor_suspend().
>>>>> What do you think about my rev6 patch?
>>>> If I understand the intention correctly, this is for the stability of
>>>> the device due to the behavior or bootloader/SoC-initializer, which
>>>> has nothing to do with governors.
>>>>
>>>> Even if users are using userspace, as long as they set the custom
>>>> frequencies lower than the default, they have the possibility of
>>>> being unstable as ondemand is going to have.
>>>>
>>>>
>>>> To reuse the update_devfreq() code, you may do something like:
>>>>
>>>> static int _update_freq(struct devfreq *devfreq, bool is_suspending)
>>>> {
>>>> /* original contents of update_freq with if statement with is_suspending wrapping get_target_freq */
>>>> }
>>>> int update_freq(struct devfreq *devfreq)
>>>> {
>>>> return _update_freq(devfreq, false);
>>>> }
>>>>
>>>>
>>>> There should be other good non-invasive methods that are not governoe-specific as well.
>>>>
>>> Thanks for your suggestion, i will update the new version soon.
>>>> Cheers,
>>>> MyungJoo
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-rockchip mailing list
>>>> Linux-rockchip at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>> --
>>> Lin Huang
>>>
>>
>>
>>
>
^ permalink raw reply
* [PATCH 1/4] serial: core: Add LED trigger support
From: Sascha Hauer @ 2016-11-24 8:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e9fc12b8-d4a7-067d-ef87-03f0aae39bd7@gmail.com>
On Wed, Nov 23, 2016 at 10:57:13AM -0800, Florian Fainelli wrote:
> On 11/23/2016 02:01 AM, Sascha Hauer wrote:
> > With this patch the serial core provides LED triggers for RX and TX.
> >
> > As the serial core layer does not know when the hardware actually sends
> > or receives characters, this needs help from the UART drivers.
>
> Looking at 8250, we call serial8250_tx_chars from __start_tx which is
> called form the uart_ops::start_tx, for RX I sort of agree, since this
> happens in interrupt handler. I suppose some drivers could actually
> queue work but not do it right away though?
RX is not the problem. When characters are received all drivers call
tty_flip_buffer_push() which could be used for LED triggering (either
directly or we replace the calls to tty_flip_buffer_push() with some
wrapper function).
The problem comes with TX. The serial core has a FIFO which gets
characters from the tty layer. There is no function which the serial
drivers could call to read from this FIFO, instead they have to open code
this.
Continuing with the 8250 driver serial8250_tx_chars() is not only called
from __start_tx(), but also from the interrupt handler. Transmission
works without the serial_core ever noticing.
>
> > The LED triggers are registered in uart_add_led_triggers() called from
> > the UART drivers which want to support LED triggers. All the driver
> > has to do then is to call uart_led_trigger_[tx|rx] to indicate
> > activity.
>
> Could we somehow remedy the lack of knowledge from the core as whether
> the HW sends/receives characters first before adding support for LED
> triggers? It would be more generic and future proof to require UART
> drivers to report to the core when they actually TX/RX, and then at the
> core level, utilize that knowledge to perform the LED trigger.
Maybe we could introduce a function to read from the TX FIFO. Having
open coded this in each and every driver is not nice anyway.
>
> Side note: are you positive using drv->dev_name is robust enough on
> systems with many different UART drivers, yet all of them being ttyS*?
If we have different UART drivers, only one of them provides ttyS*, no?
Other drivers will have to use another namespace.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v3] clk: qoriq: added ls1012a clock configuration
From: Scott Wood @ 2016-11-24 8:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479955015-37514-1-git-send-email-yuantian.tang@nxp.com>
On 11/23/2016 08:50 PM, yuantian.tang at nxp.com wrote:
> From: Tang Yuantian <Yuantian.Tang@nxp.com>
>
> Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>
> ---
> v3:
> - rebased to latest kernel and re-sorted the code
[snip]
> @@ -1316,6 +1334,7 @@ CLK_OF_DECLARE(qoriq_clockgen_2, "fsl,qoriq-clockgen-2.0", clockgen_init);
> CLK_OF_DECLARE(qoriq_clockgen_ls1021a, "fsl,ls1021a-clockgen", clockgen_init);
> CLK_OF_DECLARE(qoriq_clockgen_ls1043a, "fsl,ls1043a-clockgen", clockgen_init);
> CLK_OF_DECLARE(qoriq_clockgen_ls1046a, "fsl,ls1046a-clockgen", clockgen_init);
> +CLK_OF_DECLARE(qoriq_clockgen_ls1012a, "fsl,ls1012a-clockgen", clockgen_init);
> CLK_OF_DECLARE(qoriq_clockgen_ls2080a, "fsl,ls2080a-clockgen", clockgen_init);
You need a better sorting algorithm. :-P
-Scott
^ permalink raw reply
* [PATCH 1/4] serial: core: Add LED trigger support
From: Sascha Hauer @ 2016-11-24 8:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123100819.GA20137@kroah.com>
On Wed, Nov 23, 2016 at 11:08:19AM +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 23, 2016 at 11:01:03AM +0100, Sascha Hauer wrote:
> > With this patch the serial core provides LED triggers for RX and TX.
> >
> > As the serial core layer does not know when the hardware actually sends
> > or receives characters, this needs help from the UART drivers. The
> > LED triggers are registered in uart_add_led_triggers() called from
> > the UART drivers which want to support LED triggers. All the driver
> > has to do then is to call uart_led_trigger_[tx|rx] to indicate
> > activity.
BTW last time LED triggers were discussed
(https://patchwork.kernel.org/patch/9212885/) You and Arnd mandated the
triggers should be implemented in the tty layer. By tty layer did you
really mean the tty layer or did you mean serial_core?
We could implement it in the tty layer, but tty doesn't know when the
characters are actually sent. There could be arbitrary time passing
between a tty_operations->put_char and the character being on the wire.
Also I am not sure if we want to have LED triggers for each and every
tty in the system
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
From: hl @ 2016-11-24 8:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5836A1E0.1070707@samsung.com>
Hi Chanwoo Choi,
On 2016?11?24? 16:16, Chanwoo Choi wrote:
> Hi Lin,
>
> On 2016? 11? 24? 16:34, hl wrote:
>> Hi Chanwoo Choi,
>>
>> I think the dev_pm_opp_get_suspend_opp() have implement most of
>> the funtion, all we need is just define the node in dts, like following:
>>
>> &dmc_opp_table {
>> opp06 {
>> opp-suspend;
>> };
>> };
> Two approaches use the 'opp-suspend' property.
>
> I think that the method to support suspend-opp have to
> guarantee following conditions:
> - Support the all of devfreq's governors.
As MyungJoo Ham suggestion, i will set the suspend frequency in
devfreq_suspend_device(),
which will ingore governor.
> - Devfreq framework have the responsibility to change the
> frequency/voltage for suspend-opp. If we uses the
> new devfreq_suspend(), each devfreq device don't care
> how to support the suspend-opp. Just the developer of each
> devfreq device need to add 'opp-suspend' propet to OPP entry in DT file.
Why should support change the voltage in devfreq framework, i think it
shuold be handle in
specific driver, i think the devfreq only handle it can get the right
frequency, then pass it to
specific driver, i think the voltage should handle in the
devfreq->profile->target();
> Best Regards,
> Chanwoo Choi
>
>> so i think my way semm more simple.
>>
>> On 2016?11?24? 15:10, Chanwoo Choi wrote:
>>> + Tobias Jakobi,
>>>
>>> Hi Lin,
>>>
>>> We need to discuss how to support the suspend-opp of devfreq device.
>>> Now, there are two patch thread for suspend-opp of devfreq.
>>>
>>> The Lin's approach modify the devfreq_suspend_device() to support suspend-opp.
>>> The Tobias's approach[1] add new devfreq_suspend() and then call it on dpm_suspend()
>>> when entering the suspend state.
>>>
>>> [1] [RFC 0/4] PM / devfreq: draft for OPP suspend impl
>>> - https://patchwork.kernel.org/patch/9443323/
>>> - https://patchwork.kernel.org/patch/9443325/
>>> - https://patchwork.kernel.org/patch/9443329/
>>> - https://patchwork.kernel.org/patch/9443331/
>>>
>>> I think we need to discuss it together.
>>>
>>> Regards,
>>> Chanwoo Choi
>>>
>>> On 2016? 11? 24? 15:45, hl wrote:
>>>> Hi MyungJoo Ham,
>>>>
>>>> On 2016?11?24? 14:14, MyungJoo Ham wrote:
>>>>> On Thu, Nov 24, 2016 at 11:18 AM, hl <hl@rock-chips.com> wrote:
>>>>>> Hi MyungJoo Ham,
>>>>> []
>>>>>>> We still need to sync the all status even i call target() in
>>>>>>> devfreq_suspend/resume_device
>>>>>>> directly, so still need update_devfreq() other setp except
>>>>>>> devfreq->governor->get_target_freq(devfreq, &freq);
>>>>>> And i think it better to be governor behaviors, for userspace they may not
>>>>>> want to change
>>>>>> the suspend frequency like other governor, the frequency should decide by
>>>>>> the user, if they
>>>>>> want this function, they should like other governor to rigister a
>>>>>> devfreq_monitor_suspend().
>>>>>> What do you think about my rev6 patch?
>>>>> If I understand the intention correctly, this is for the stability of
>>>>> the device due to the behavior or bootloader/SoC-initializer, which
>>>>> has nothing to do with governors.
>>>>>
>>>>> Even if users are using userspace, as long as they set the custom
>>>>> frequencies lower than the default, they have the possibility of
>>>>> being unstable as ondemand is going to have.
>>>>>
>>>>>
>>>>> To reuse the update_devfreq() code, you may do something like:
>>>>>
>>>>> static int _update_freq(struct devfreq *devfreq, bool is_suspending)
>>>>> {
>>>>> /* original contents of update_freq with if statement with is_suspending wrapping get_target_freq */
>>>>> }
>>>>> int update_freq(struct devfreq *devfreq)
>>>>> {
>>>>> return _update_freq(devfreq, false);
>>>>> }
>>>>>
>>>>>
>>>>> There should be other good non-invasive methods that are not governoe-specific as well.
>>>>>
>>>> Thanks for your suggestion, i will update the new version soon.
>>>>> Cheers,
>>>>> MyungJoo
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Linux-rockchip mailing list
>>>>> Linux-rockchip at lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>> --
>>>> Lin Huang
>>>>
>>>
>>>
>
>
>
--
Lin Huang
^ permalink raw reply
* [RFC PATCH] ARM: dts: Add support for Turris Omnia
From: Uwe Kleine-König @ 2016-11-24 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123003505.GL2691@lunn.ch>
On 11/23/2016 01:35 AM, Andrew Lunn wrote:
>> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
>> @@ -0,0 +1,279 @@
>> +/*
>> + * Device Tree file for the Turris Omnia
>> + * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf
>
> Cool that there is a link to the schematics. But please could you put
> it lower down. It is more likely to be seen if it comes after the
> copyright and license section.
I added to the top because that's where I would look. But checking other
dts files it seems indeed to be more common after the copyright stuff.
I'd suggest to even start a new comment (i.e.
* last blabla of copyright
*/
/*
* Schematic available at ...
) to be more "loud".
@Tomas: I think it doesn't make sense when we alternate sending patches
without prior arrangement. Do you already work on a v5? If not I can do
that to fix the last few comments. Not sure when a submission is too
late to enter v4.10, but I think the window isn't that big any more.
> No leds? No buttons via gpio-keys?
The leds are controlled by a Cortex-M0 and without intervention blink
according to a hardware function (network, power, pci). IMHO that's ok
for an initial setup.
And there are no buttons that are routed to the Armada CPU. Just a reset
button (well, ok, this one is routed to the Armada CPU, but you cannot
make this a gpio-key :-) and the other button is used to control the
brightness of the LEDs and is only routed to the M0.
Best regards
Uwe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161124/8bd849c4/attachment-0001.sig>
^ permalink raw reply
* [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible
From: Jisheng Zhang @ 2016-11-24 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPv3WKf0a63qQT+xwXfUatbgLFF58e6L8J10VtBOUTam+kUcjg@mail.gmail.com>
Hi Marcin, Gregory, Arnd,
On Wed, 23 Nov 2016 17:02:11 +0100 Marcin Wojtas wrote:
> Hi Gregory,
>
> 2016-11-23 14:07 GMT+01:00 Gregory CLEMENT:
> > Hi Jisheng, Arnd,
> >
> >
> > Thanks for your feedback.
> >
> >
> > On mer., nov. 23 2016, Arnd Bergmann wrote:
> >
> >> On Wednesday, November 23, 2016 5:53:41 PM CET Jisheng Zhang wrote:
> >>> On Tue, 22 Nov 2016 22:04:12 +0100 Arnd Bergmann wrote:
> >>>
> >>> > On Tuesday, November 22, 2016 5:48:41 PM CET Gregory CLEMENT wrote:
> >>> > > +#ifdef CONFIG_64BIT
> >>> > > + void *data_tmp;
> >>> > > +
> >>> > > + /* In Neta HW only 32 bits data is supported, so in order to
> >>> > > + * obtain whole 64 bits address from RX descriptor, we store
> >>> > > + * the upper 32 bits when allocating buffer, and put it back
> >>> > > + * when using buffer cookie for accessing packet in memory.
> >>> > > + * Frags should be allocated from single 'memory' region,
> >>> > > + * hence common upper address half should be sufficient.
> >>> > > + */
> >>> > > + data_tmp = mvneta_frag_alloc(pp->frag_size);
> >>> > > + if (data_tmp) {
> >>> > > + pp->data_high = (u64)upper_32_bits((u64)data_tmp) << 32;
> >>> > > + mvneta_frag_free(pp->frag_size, data_tmp);
> >>> > > + }
> >>> > >
> >>> >
> >>> > How does this work when the region spans a n*4GB address boundary?
> >>>
> >>> indeed. We also make use of this driver on 64bit platforms. We use
> >>> different solution to make the driver 64bit safe.
> >>>
> >>> solA: make use of the reserved field in the mvneta_rx_desc, such
> >>> as reserved2 etc. Yes, the field is marked as "for future use, PnC", but
> >>> now it's not used at all. This is one possible solution however.
> >>
> >> Right, this sounds like the most straightforward choice.
> >
> > The PnC (which stands for Parsing and Classification) is not used yet
> > indeed but this field will be needed when we will enable it. It is
> > something we want to do but it is not planned in a near future. However
> > from the datasheets I have it seems only present on the Armada XP. It is
> > not mentioned on datasheets for the Armada 38x or the Armada 3700.
> >
>
> It is not mentioned in A38x spec, but this SoC has exactly the same
> PnC as Armada XP (they differ only with used SRAM details). I wouldn't
> be surprised if it was supported on A3700 as well.
>
> > That would mean it was safe to use on of this field in 64-bits mode on
> > the Armada 3700.
> >
> > So I am going to take this approach.
> >
>
> I think for now it's safe and is much easier than handling extra
> software ring for virtual addresses.
>
solB (a SW shadow cookie) perhaps gives a better performance: in hot path,
such as mvneta_rx(), the driver accesses buf_cookie and buf_phys_addr of
rx_desc which is allocated by dma_alloc_coherent, it's noncacheable if the
device isn't cache-coherent. I didn't measure the performance difference,
because in fact we take solA as well internally. From your experience,
can the performance gain deserve the complex code?
Thanks,
Jisheng
^ permalink raw reply
* [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: Bartosz Golaszewski @ 2016-11-24 8:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20fbc946-d56c-31a3-4ae7-cf61df96a3c3@ti.com>
2016-11-24 6:03 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Thursday 24 November 2016 04:18 AM, David Lechner wrote:
>> On 11/23/2016 04:32 PM, Kevin Hilman wrote:
>>> David Lechner <david@lechnology.com> writes:
>>>
>>>> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote:
>>>>> 2016-11-22 23:23 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote:
>>>>>>>
>>>>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
>>>>>>> controller drivers to da850.dtsi.
>>>>>>>
>>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>>> ---
>>>>>>> v1 -> v2:
>>>>>>> - moved the priority controller node above the cfgchip node
>>>>>>> - renamed added nodes to better reflect their purpose
>>>>>>>
>>>>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>>>>>> 1 file changed, 8 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi
>>>>>>> b/arch/arm/boot/dts/da850.dtsi
>>>>>>> index 1bb1f6d..412eec6 100644
>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>> @@ -210,6 +210,10 @@
>>>>>>> };
>>>>>>>
>>>>>>> };
>>>>>>> + prictrl: priority-controller at 14110 {
>>>>>>> + compatible = "ti,da850-mstpri";
>>>>>>> + reg = <0x14110 0x0c>;
>>>>>>
>>>>>>
>>>>>> I think we should add status = "disabled"; here and let boards opt in.
>>>>>>
>>>>>>> + };
>>>>>>> cfgchip: chip-controller at 1417c {
>>>>>>> compatible = "ti,da830-cfgchip", "syscon",
>>>>>>> "simple-mfd";
>>>>>>> reg = <0x1417c 0x14>;
>>>>>>> @@ -451,4 +455,8 @@
>>>>>>> 1 0 0x68000000 0x00008000>;
>>>>>>> status = "disabled";
>>>>>>> };
>>>>>>> + memctrl: memory-controller at b0000000 {
>>>>>>> + compatible = "ti,da850-ddr-controller";
>>>>>>> + reg = <0xb0000000 0xe8>;
>>>>>>
>>>>>>
>>>>>> same here. status = "disabled";
>>>>>>
>>>>>>> + };
>>>>>>> };
>>>>>>>
>>>>>
>>>>> Hi David,
>>>>>
>>>>> I did that initially[1][2] and it was rejected by Kevin[3] and
>>>>> Laurent[4].
>>>>>
>>>>> FYI this patch has already been queued by Sekhar.
>>>>
>>>> Thanks. I did not see those threads.
>>>>
>>>> FYI to maintainers, having these enabled by default causes error
>>>> messages in the kernel log for other boards that are not supported by
>>>> the drivers.
>>>
>>> Then the driver is too noisy and should be cleaned up.
>>>
>>>> Since there is only one board that is supported and soon
>>>> to be 2 that are not, I would rather have this disabled by default to
>>>> avoid the error messages.
>>>
>>> IMO, what exactly are the error messages? Sounds like the driver is
>>> being too verbose, and calling things errors that are not really errors.
>>
>> It is just one line per driver.
>>
>> dev_err(dev, "no master priorities defined for this board\n");
>>
>> and
>>
>> dev_err(dev, "no settings defined for this board\n");
>>
>>
>> Since "ti,da850-lcdk" is the only board supported in these drivers, all
>> other boards will see these error messages.
>
> Thats pretty bad. Sorry about that. The original justification for
> keeping them enabled all the time was that they are in-SoC modules with
> no external dependencies (like IO lines or voltage rails) so they can be
> enabled on all boards that use DA850. While that remains true, the
> configuration itself is board specific.
>
> I think the error messages are still useful, so instead of silencing
> them, I think we should go back to keeping these nodes disabled by
> default and enabling only on boards which have support for it in the driver.
>
> Thanks,
> Sekhar
I'll send a patch.
Thanks,
Bartosz
^ permalink raw reply
* [PATCH 1/7] add binding for stm32 multifunctions timer driver
From: Lee Jones @ 2016-11-24 8:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks6rnago88JC0C5Uj6JpGZGuwyoOj-W8+r7Oj4s8_GuXyA@mail.gmail.com>
Rob,
Would you mind casting an eye on this please?
On Wed, 23 Nov 2016, Benjamin Gaignard wrote:
> 2016-11-23 10:21 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
> > On Wed, 23 Nov 2016, Benjamin Gaignard wrote:
> >
> >> 2016-11-22 17:52 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
> >> > On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> >> >
> >> >> Add bindings information for stm32 timer MFD
> >> >>
> >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> >> ---
> >> >> .../devicetree/bindings/mfd/stm32-timer.txt | 53 ++++++++++++++++++++++
> >> >> 1 file changed, 53 insertions(+)
> >> >> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >> new file mode 100644
> >> >> index 0000000..3cefce1
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >> @@ -0,0 +1,53 @@
> >> >> +STM32 multifunctions timer driver
> >> >
> >> > "STM32 Multi-Function Timer/PWM device bindings"
> >> >
> >> > Doesn't this shared device have a better name?
> >>
> >> In SoC documentation those hardware blocks are named "advanced-control
> >> timers", "general purpose timers" or "basic timers"
> >> "stm32-timer" name is already used for clock source driver, that why I
> >> have prefix it with mfd
> >
> > MFD is a Linuxisum and has no place in hardware description.
> >
> > Please used one of the names you mentioned above.
>
> I will go for "st,stm32-advanced-timer"
>
> >
> > Hopefully the one that best fits.
> >
> >> >> +stm32 timer MFD allow to handle at the same time pwm and IIO timer devices
> >> >
> >> > No need for this sentence.
> >> >
> >> OK
> >>
> >> >> +Required parameters:
> >> >> +- compatible: must be one of the follow value:
> >> >> + "st,stm32-mfd-timer1"
> >> >> + "st,stm32-mfd-timer2"
> >> >> + "st,stm32-mfd-timer3"
> >> >> + "st,stm32-mfd-timer4"
> >> >> + "st,stm32-mfd-timer5"
> >> >> + "st,stm32-mfd-timer6"
> >> >> + "st,stm32-mfd-timer7"
> >> >> + "st,stm32-mfd-timer8"
> >> >> + "st,stm32-mfd-timer9"
> >> >> + "st,stm32-mfd-timer10"
> >> >> + "st,stm32-mfd-timer11"
> >> >> + "st,stm32-mfd-timer12"
> >> >> + "st,stm32-mfd-timer13"
> >> >> + "st,stm32-mfd-timer14"
> >> >
> >> > We don't normally number devices.
> >> >
> >> > What's stopping you from simply doing:
> >> >
> >> > pwm1: pwm1 at 40010000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> > pwm2: pwm1 at 40020000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> > pwm3: pwm1 at 40030000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> >
> >>
> >> Because each instance of the hardware is slightly different: number of
> >> pwm channels, triggers capabilities, etc ..
> >> so I need to distinguish them.
> >> Since it look to be a problem I will follow your suggestion and add a
> >> property this driver to be able to identify each instance.
> >> Do you think that "id" parameter (integer for 1 to 14) is acceptable ?
> >
> > Unfortunately not. IDs aren't allowed in DT.
> >
> > What about "pwm-chans" and "trigger"?
> >
> > pwm-chans : Number of available channels available
>
> For pwm I need those 4 properties:
> st,pwm-number: the number of PWM devices
st,pwm-num-chan is already documented.
Please use that instead of creating new properties.
> st,complementary: if exist have complementary ouput
> st,32bit-counter: if exist have 32 bits counter
> st,breakinput-polarity: if set enable break input feature.
>
> Is it acceptable from pwm maintainer point of view ?
>
> > trigger : Boolean value specifying whether a timer is present
>
> Following our discussion on IRC I will try to code for your proposal:
>
> advanced-timer at 40010000 {
> compatible = "st,stm32-advanced-timer";
> reg = <0x40010000 0x400>;
> clocks = <&rcc 0 160>;
> clock-names = "clk_int";
>
> pwm at 0 {
> compatible = "st,stm32-pwm";
> st,pwm-number= <4>;
> st,complementary;
> st,breakinput;
> };
>
> timer at 0 {
> reg = <1>;
> compatible = "st,stm32-iio-timer";
> interrupts = <27>;
> triggers = <5 2 3 4>;
> };
> };
>
> triggers parameter will be used to know which trigger are valid for
> the IIO device
Except for "st,pwm-number" as mentioned above, this looks good to me.
Rob, would what do you think?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible
From: Arnd Bergmann @ 2016-11-24 9:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124163327.1cc261ab@xhacker>
On Thursday, November 24, 2016 4:37:36 PM CET Jisheng Zhang wrote:
> solB (a SW shadow cookie) perhaps gives a better performance: in hot path,
> such as mvneta_rx(), the driver accesses buf_cookie and buf_phys_addr of
> rx_desc which is allocated by dma_alloc_coherent, it's noncacheable if the
> device isn't cache-coherent. I didn't measure the performance difference,
> because in fact we take solA as well internally. From your experience,
> can the performance gain deserve the complex code?
Yes, a read from uncached memory is fairly slow, so if you have a chance
to avoid that it will probably help. When adding complexity to the code,
it probably makes sense to take a runtime profile anyway quantify how
much it gains.
On machines that have cache-coherent DMA, accessing the descriptor
should be fine, as you already have to load the entire cache line
to read the status field.
Looking at this snippet:
rx_status = rx_desc->status;
rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
data = (unsigned char *)rx_desc->buf_cookie;
phys_addr = rx_desc->buf_phys_addr;
pool_id = MVNETA_RX_GET_BM_POOL_ID(rx_desc);
bm_pool = &pp->bm_priv->bm_pools[pool_id];
if (!mvneta_rxq_desc_is_first_last(rx_status) ||
(rx_status & MVNETA_RXD_ERR_SUMMARY)) {
err_drop_frame_ret_pool:
/* Return the buffer to the pool */
mvneta_bm_pool_put_bp(pp->bm_priv, bm_pool,
rx_desc->buf_phys_addr);
err_drop_frame:
I think there is more room for optimizing if you start: you read
the status field twice (the second one in MVNETA_RX_GET_BM_POOL_ID)
and you can cache the buf_phys_addr along with the virtual address
once you add that.
Generally speaking, I'd recommend using READ_ONCE()/WRITE_ONCE()
to access the descriptor fields, to ensure the compiler doesn't
add extra references as well as to annotate the expensive
operations.
Arnd
^ permalink raw reply
* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ulf Hansson @ 2016-11-24 9:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87d1hno2d7.fsf@free-electrons.com>
On 22 November 2016 at 18:23, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Rob,
>
> On jeu., nov. 10 2016, Ziji Hu <huziji@marvell.com> wrote:
>
> [...]
>
>>>> +
>>>> +- reg:
>>>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>>> +
>>>> + * For "marvell,armada-3700-sdhci", two register areas.
>>>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>>> + PHY PAD Voltage Control register.
>>>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>>> + in below.
>>>> + Please also check property marvell,pad-type in below.
>>>> +
>>>> +Optional Properties:
>>>> +- marvell,xenon-slotno:
>>>
>>> Multiple slots should be represented as child nodes IMO. I think some
>>> other bindings already do this.
>>>
>>
>> All the slots are entirely independent.
>> I prefer to consider it as multiple independent SDHCs placed in
>> a single IP, instead of that a IP contains multiple child slots.
>
> It was indeed what I tried to show in my answer for the 1st version:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html
>
> Maybe you missed it.
>
> You also mentioned other bindings using child nodes, but for this one
> we have one controller with only one set of register with multiple slots
> (Atmel is an example). Here each slot have it own set of register.
>
> Actually giving the fact that each slot is controlled by a different set
> of register I wonder why the hardware can't also deduce the slot number
> from the address register. For me it looks like an hardware bug but we
> have to deal with it.
>
> Do you still think we needchild node here?
Using child-nodes for slots like what's done in the atmel case, is
currently broken. I would recommend to avoid using child-nodes for
slots, if possible.
To give you some more background, currently the mmc core treats child
nodes as embedded non-removable cards or SDIO funcs. However, we can
change to make child-nodes also allowed to describe slots, but it
requires a specific compatible for "slots" and of course then we also
need to update the DT parsing of the child-nodes in the mmc core.
Documentation/devicetree/bindings/mmc/mmc.txt
Documentation/devicetree/bindings/mmc/mmc-card.txt
>
>>
>> It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
>> If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
>> In my very own opinion, it is inconvenient and unnecessary.
>
Kind regards
Uffe
^ permalink raw reply
* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Arnd Bergmann @ 2016-11-24 9:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFpoifsKkse7Fc-bbZAoa=QGT=9QOQ-4D=f60ptx0hzZsA@mail.gmail.com>
On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
> > You also mentioned other bindings using child nodes, but for this one
> > we have one controller with only one set of register with multiple slots
> > (Atmel is an example). Here each slot have it own set of register.
> >
> > Actually giving the fact that each slot is controlled by a different set
> > of register I wonder why the hardware can't also deduce the slot number
> > from the address register. For me it looks like an hardware bug but we
> > have to deal with it.
> >
> > Do you still think we needchild node here?
>
> Using child-nodes for slots like what's done in the atmel case, is
> currently broken. I would recommend to avoid using child-nodes for
> slots, if possible.
>
> To give you some more background, currently the mmc core treats child
> nodes as embedded non-removable cards or SDIO funcs. However, we can
> change to make child-nodes also allowed to describe slots, but it
> requires a specific compatible for "slots" and of course then we also
> need to update the DT parsing of the child-nodes in the mmc core.
>
> Documentation/devicetree/bindings/mmc/mmc.txt
> Documentation/devicetree/bindings/mmc/mmc-card.txt
I don't see anything wrong with having child nodes for the slots
even with the current binding, under one condition:
The mmc.txt binding above must refer only to the child node, while
the parent node conceptually becomes a plain bus or MFD that
happens to encapsulate multiple MMC host controllers, and possibly
provides some shared registers to them.
Arnd
^ permalink raw reply
* [PATCH v7] PM/devfreq: add suspend frequency support
From: Lin Huang @ 2016-11-24 9:11 UTC (permalink / raw)
To: linux-arm-kernel
Add suspend frequency support and if needed set it to
the frequency obtained from the suspend opp (can be defined
using opp-v2 bindings and is optional).
Signed-off-by: Lin Huang <hl@rock-chips.com>
---
Changes in v2:
- use update_devfreq() instead devfreq_update_status()
Changes in v3:
- fix build error
Changes in v4:
- move dev_pm_opp_get_suspend_opp() to devfreq_add_device()
Changes in v5:
- delete devfreq_opp_get_suspend_opp() in devfreq.h
Changes in v6:
- return to use stop_polling check suspend status
Changes in v7:
- move handle suspend frequency in devfreq_suspend_device()
drivers/devfreq/devfreq.c | 47 ++++++++++++++++++++++++++++++++++-------------
include/linux/devfreq.h | 1 +
2 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index da72d97..958abc8 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -212,16 +212,7 @@ static int devfreq_notify_transition(struct devfreq *devfreq,
return 0;
}
-/* Load monitoring helper functions for governors use */
-
-/**
- * update_devfreq() - Reevaluate the device and configure frequency.
- * @devfreq: the devfreq instance.
- *
- * Note: Lock devfreq->lock before calling update_devfreq
- * This function is exported for governors.
- */
-int update_devfreq(struct devfreq *devfreq)
+static int _update_devfreq(struct devfreq *devfreq, bool is_suspending)
{
struct devfreq_freqs freqs;
unsigned long freq, cur_freq;
@@ -237,9 +228,13 @@ int update_devfreq(struct devfreq *devfreq)
return -EINVAL;
/* Reevaluate the proper frequency */
- err = devfreq->governor->get_target_freq(devfreq, &freq);
- if (err)
- return err;
+ if (is_suspending && devfreq->suspend_freq) {
+ freq = devfreq->suspend_freq;
+ } else {
+ err = devfreq->governor->get_target_freq(devfreq, &freq);
+ if (err)
+ return err;
+ }
/*
* Adjust the frequency with user freq and QoS.
@@ -285,6 +280,21 @@ int update_devfreq(struct devfreq *devfreq)
devfreq->previous_freq = freq;
return err;
}
+
+/* Load monitoring helper functions for governors use */
+
+/**
+ * update_devfreq() - Reevaluate the device and configure frequency.
+ * @devfreq: the devfreq instance.
+ *
+ * Note: Lock devfreq->lock before calling update_devfreq
+ * This function is exported for governors.
+ */
+
+int update_devfreq(struct devfreq *devfreq)
+{
+ return _update_devfreq(devfreq, false);
+}
EXPORT_SYMBOL(update_devfreq);
/**
@@ -524,6 +534,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
struct devfreq *devfreq;
struct devfreq_governor *governor;
int err = 0;
+ struct dev_pm_opp *suspend_opp;
if (!dev || !profile || !governor_name) {
dev_err(dev, "%s: Invalid parameters.\n", __func__);
@@ -558,6 +569,12 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->data = data;
devfreq->nb.notifier_call = devfreq_notifier_call;
+ rcu_read_lock();
+ suspend_opp = dev_pm_opp_get_suspend_opp(dev);
+ if (suspend_opp)
+ devfreq->suspend_freq = dev_pm_opp_get_freq(suspend_opp);
+ rcu_read_unlock();
+
if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
mutex_unlock(&devfreq->lock);
devfreq_set_freq_table(devfreq);
@@ -754,6 +771,10 @@ int devfreq_suspend_device(struct devfreq *devfreq)
if (!devfreq->governor)
return 0;
+ mutex_lock(&devfreq->lock);
+ _update_devfreq(devfreq, true);
+ mutex_unlock(&devfreq->lock);
+
return devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_SUSPEND, NULL);
}
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 98c6993..517e394 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -172,6 +172,7 @@ struct devfreq {
struct delayed_work work;
unsigned long previous_freq;
+ unsigned long suspend_freq;
struct devfreq_dev_status last_status;
void *data; /* private data for governors */
--
2.6.6
^ permalink raw reply related
* [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible
From: Jisheng Zhang @ 2016-11-24 9:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <21520380.oWTKcrq8DS@wuerfel>
On Thu, 24 Nov 2016 10:00:36 +0100
Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday, November 24, 2016 4:37:36 PM CET Jisheng Zhang wrote:
> > solB (a SW shadow cookie) perhaps gives a better performance: in hot path,
> > such as mvneta_rx(), the driver accesses buf_cookie and buf_phys_addr of
> > rx_desc which is allocated by dma_alloc_coherent, it's noncacheable if the
> > device isn't cache-coherent. I didn't measure the performance difference,
> > because in fact we take solA as well internally. From your experience,
> > can the performance gain deserve the complex code?
>
> Yes, a read from uncached memory is fairly slow, so if you have a chance
> to avoid that it will probably help. When adding complexity to the code,
> it probably makes sense to take a runtime profile anyway quantify how
> much it gains.
>
> On machines that have cache-coherent DMA, accessing the descriptor
> should be fine, as you already have to load the entire cache line
> to read the status field.
>
> Looking at this snippet:
>
> rx_status = rx_desc->status;
> rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
> data = (unsigned char *)rx_desc->buf_cookie;
> phys_addr = rx_desc->buf_phys_addr;
> pool_id = MVNETA_RX_GET_BM_POOL_ID(rx_desc);
> bm_pool = &pp->bm_priv->bm_pools[pool_id];
>
> if (!mvneta_rxq_desc_is_first_last(rx_status) ||
> (rx_status & MVNETA_RXD_ERR_SUMMARY)) {
> err_drop_frame_ret_pool:
> /* Return the buffer to the pool */
> mvneta_bm_pool_put_bp(pp->bm_priv, bm_pool,
> rx_desc->buf_phys_addr);
> err_drop_frame:
>
>
> I think there is more room for optimizing if you start: you read
> the status field twice (the second one in MVNETA_RX_GET_BM_POOL_ID)
> and you can cache the buf_phys_addr along with the virtual address
> once you add that.
oh, yeah! buf_phy_addr could be included too.
>
> Generally speaking, I'd recommend using READ_ONCE()/WRITE_ONCE()
> to access the descriptor fields, to ensure the compiler doesn't
> add extra references as well as to annotate the expensive
> operations.
>
> Arnd
Got it. Thanks so much for the detailed guide.
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-11-24 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4675465.4Qhqy6WU4X@wuerfel>
Hi, Arnd,
Thanks you very much!
To understand your idea more clear, I have some questions on your patch sketch.
Please check it below.
On 2016/11/24 7:23, Arnd Bergmann wrote:
> On Wednesday, November 23, 2016 6:07:11 PM CET Arnd Bergmann wrote:
>> On Wednesday, November 23, 2016 3:22:33 PM CET Gabriele Paoloni wrote:
>>> From: Arnd Bergmann [mailto:arnd at arndb.de]
>>>> On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote:
>>
>> Please don't proliferate the use of
>> pci_pio_to_address/pci_address_to_pio here, computing the physical
>> address from the logical address is trivial, you just need to
>> subtract the start of the range that you already use when matching
>> the port number range.
>>
>> The only thing we need here is to make of_address_to_resource()
>> return the correct logical port number that was registered for
>> a given host device when asked to translate an address that
>> does not have a CPU address associated with it.
>
> Ok, I admit this was a little harder than I expected, but see below
> for a rough outline of how I think it can be done.
>
> This makes it possible to translate bus specific I/O port numbers
> from device nodes into Linux port numbers, and gives a way to register
> them. We could take this further and completely remove pci_pio_to_address
> and pci_address_to_pio if we make the I/O port translation always
> go through the io_range list, looking up up the hostbridge by fwnode,
> but we don't have to do that now.
>
> The patch is completely untested and probably buggy, it just seemed
> easier to put out a prototype than to keep going in circles with the
> discussion.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index bf601d4df8cf..6cadf0501bb0 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -730,7 +730,8 @@ static void acpi_pci_root_validate_resources(struct device *dev,
> }
> }
>
> -static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
> +static void acpi_pci_root_remap_iospace(struct fwnode_handle *node,
> + struct resource_entry *entry)
> {
> #ifdef PCI_IOBASE
> struct resource *res = entry->res;
> @@ -739,11 +740,7 @@ static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
> resource_size_t length = resource_size(res);
> unsigned long port;
>
> - if (pci_register_io_range(cpu_addr, length))
> - goto err;
> -
> - port = pci_address_to_pio(cpu_addr);
> - if (port == (unsigned long)-1)
> + if (pci_register_io_range(node, cpu_addr, length, &port))
> goto err;
>
> res->start = port;
> @@ -781,7 +778,8 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
> else {
> resource_list_for_each_entry_safe(entry, tmp, list) {
> if (entry->res->flags & IORESOURCE_IO)
> - acpi_pci_root_remap_iospace(entry);
> + acpi_pci_root_remap_iospace(&device->fwnode,
> + entry);
>
> if (entry->res->flags & IORESOURCE_DISABLED)
> resource_list_destroy_entry(entry);
I think those changes in pci_root.c is only to match the new definition of
pci_register_io_range() and work for PCI I/O. It doesn't make sense for LPC, is
it right?
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index a50025a3777f..df96955a43f8 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -760,8 +760,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
> set_bit(NBD_RUNNING, &nbd->runtime_flags);
> blk_mq_update_nr_hw_queues(&nbd->tag_set, nbd->num_connections);
> args = kcalloc(num_connections, sizeof(*args), GFP_KERNEL);
> - if (!args)
> + if (!args) {
> + error = -ENOMEM;
> goto out_err;
> + }
> nbd->task_recv = current;
> mutex_unlock(&nbd->config_lock);
>
I think change here is none of the business.:)
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903fe9d2..5decaba96eed 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -2,6 +2,7 @@
> #define pr_fmt(fmt) "OF: " fmt
>
> #include <linux/device.h>
> +#include <linux/fwnode.h>
> #include <linux/io.h>
> #include <linux/ioport.h>
> #include <linux/module.h>
> @@ -323,14 +324,9 @@ int of_pci_range_to_resource(struct of_pci_range *range,
>
> if (res->flags & IORESOURCE_IO) {
> unsigned long port;
> - err = pci_register_io_range(range->cpu_addr, range->size);
> + err = pci_register_io_range(&np->fwnode, range->cpu_addr, range->size, &port);
> if (err)
> goto invalid_range;
> - port = pci_address_to_pio(range->cpu_addr);
> - if (port == (unsigned long)-1) {
> - err = -EINVAL;
> - goto invalid_range;
> - }
> res->start = port;
> } else {
> if ((sizeof(resource_size_t) < 8) &&
> @@ -479,7 +475,7 @@ static int of_empty_ranges_quirk(struct device_node *np)
> return false;
> }
>
> -static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> +static u64 of_translate_one(struct device_node *parent, struct of_bus *bus,
> struct of_bus *pbus, __be32 *addr,
> int na, int ns, int pna, const char *rprop)
> {
> @@ -507,7 +503,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> ranges = of_get_property(parent, rprop, &rlen);
> if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> pr_debug("no ranges; cannot translate\n");
> - return 1;
> + return OF_BAD_ADDR;
> }
> if (ranges == NULL || rlen == 0) {
> offset = of_read_number(addr, na);
> @@ -528,7 +524,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> }
> if (offset == OF_BAD_ADDR) {
> pr_debug("not found !\n");
> - return 1;
> + return offset;
> }
> memcpy(addr, ranges + na, 4 * pna);
>
> @@ -537,7 +533,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> pr_debug("with offset: %llx\n", (unsigned long long)offset);
>
> /* Translate it into parent bus space */
> - return pbus->translate(addr, offset, pna);
> + if (pbus->translate(addr, offset, pna))
> + return OF_BAD_ADDR;
> +
> + return offset;
> }
>
> /*
> @@ -549,9 +548,14 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> * that translation is impossible (that is we are not dealing with a value
> * that can be mapped to a cpu physical address). This is not really specified
> * that way, but this is traditionally the way IBM at least do things
> + *
> + * Whenever the translation fails, the *host pointer will be set to the
> + * device that lacks a tranlation, and the return code is relative to
> + * that node.
> */
> static u64 __of_translate_address(struct device_node *dev,
> - const __be32 *in_addr, const char *rprop)
> + const __be32 *in_addr, const char *rprop,
> + struct device_node **host)
> {
> struct device_node *parent = NULL;
> struct of_bus *bus, *pbus;
> @@ -564,6 +568,7 @@ static u64 __of_translate_address(struct device_node *dev,
> /* Increase refcount at current level */
> of_node_get(dev);
>
> + *host = NULL;
> /* Get parent & match bus type */
> parent = of_get_parent(dev);
> if (parent == NULL)
> @@ -600,8 +605,9 @@ static u64 __of_translate_address(struct device_node *dev,
> pbus = of_match_bus(parent);
> pbus->count_cells(dev, &pna, &pns);
> if (!OF_CHECK_COUNTS(pna, pns)) {
> - pr_err("Bad cell count for %s\n",
> - of_node_full_name(dev));
> + pr_debug("Bad cell count for %s\n",
> + of_node_full_name(dev));
> + *host = of_node_get(parent);
> break;
> }
I don't think here is the right place to fill *host. I think you want to return
the parent where the of_translate_one() failed for the 'ranges' property
missing. So, I think this seems better:
if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop)) {
*host = of_node_get(dev);
break;
}
>
> @@ -609,7 +615,9 @@ static u64 __of_translate_address(struct device_node *dev,
> pbus->name, pna, pns, of_node_full_name(parent));
>
> /* Apply bus translation */
> - if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop))
> + result = of_translate_one(dev, bus, pbus, addr, na, ns,
> + pna, rprop);
> + if (result == OF_BAD_ADDR)
> break;
>
> /* Complete the move up one level */
> @@ -628,13 +636,32 @@ static u64 __of_translate_address(struct device_node *dev,
>
> u64 of_translate_address(struct device_node *dev, const __be32 *in_addr)
> {
> - return __of_translate_address(dev, in_addr, "ranges");
> + struct device_node *host;
> + u64 ret;
> +
> + ret = __of_translate_address(dev, in_addr, "ranges", &host);
> + if (host) {
> + of_node_put(host);
> + return OF_BAD_ADDR;
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL(of_translate_address);
>
> u64 of_translate_dma_address(struct device_node *dev, const __be32 *in_addr)
> {
> - return __of_translate_address(dev, in_addr, "dma-ranges");
> + struct device_node *host;
> + u64 ret;
> +
> + ret = __of_translate_address(dev, in_addr, "dma-ranges", &host);
> +
> + if (host) {
> + of_node_put(host);
> + return OF_BAD_ADDR;
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL(of_translate_dma_address);
>
> @@ -676,29 +703,48 @@ const __be32 *of_get_address(struct device_node *dev, int index, u64 *size,
> }
> EXPORT_SYMBOL(of_get_address);
>
> +extern unsigned long extio_translate(struct fwnode_handle *node, unsigned long offset);
> +
> +u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr)
> +{
> + u64 taddr;
> + unsigned long port;
> + struct device_node *host;
> +
> + taddr = __of_translate_address(dev, in_addr, "ranges", &host);
> + if (host) {
> + /* host specific port access */
> + port = extio_translate(&host->fwnode, taddr);
> + of_node_put(host);
> + } else {
> + /* memory mapped I/O range */
> + port = pci_address_to_pio(taddr);
> + if (port == (unsigned long)-1)
> + return OF_BAD_ADDR;
> + }
> +
> + return port;
> +}
> +
> static int __of_address_to_resource(struct device_node *dev,
> const __be32 *addrp, u64 size, unsigned int flags,
> const char *name, struct resource *r)
> {
> u64 taddr;
>
> - if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
> + if (flags & IORESOURCE_MEM)
> + taddr = of_translate_address(dev, addrp);
> + else if (flags & IORESOURCE_IO)
> + taddr = of_translate_ioport(dev, addrp);
> + else
> return -EINVAL;
> - taddr = of_translate_address(dev, addrp);
> +
> if (taddr == OF_BAD_ADDR)
> return -EINVAL;
> memset(r, 0, sizeof(struct resource));
> - if (flags & IORESOURCE_IO) {
> - unsigned long port;
> - port = pci_address_to_pio(taddr);
> - if (port == (unsigned long)-1)
> - return -EINVAL;
> - r->start = port;
> - r->end = port + size - 1;
> - } else {
> - r->start = taddr;
> - r->end = taddr + size - 1;
> - }
> +
> + r->start = taddr;
> + r->end = taddr + size - 1;
> r->flags = flags;
> r->name = name ? name : dev->full_name;
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index eda6a7cf0e54..320ab9fbf6af 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3249,6 +3249,7 @@ EXPORT_SYMBOL(pci_request_regions_exclusive);
> #ifdef PCI_IOBASE
> struct io_range {
> struct list_head list;
> + struct fwnode_handle *node;
> phys_addr_t start;
> resource_size_t size;
> };
> @@ -3257,11 +3258,14 @@ static LIST_HEAD(io_range_list);
> static DEFINE_SPINLOCK(io_range_lock);
> #endif
>
> +#define IO_RANGE_IOEXT (resource_size_t)(-1ull)
> +
> /*
> * Record the PCI IO range (expressed as CPU physical address + size).
> * Return a negative value if an error has occured, zero otherwise
> */
> -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> +int __weak pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr,
> + resource_size_t size, unsigned long *port)
> {
> int err = 0;
>
> @@ -3272,7 +3276,12 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> /* check if the range hasn't been previously recorded */
> spin_lock(&io_range_lock);
> list_for_each_entry(range, &io_range_list, list) {
> - if (addr >= range->start && addr + size <= range->start + size) {
> + if (node == range->node)
> + goto end_register;
> +
I don't think it is safe to only check the node had been registered. For
PCI/PCIE, there is only one I/O windows in bridge, it seems ok. But for non-pci
devices, such as ISA/LPC, I wonder it is possible there are several disjoint I/O
'ranges' entries...
What parameters are necessary for linux PIO allocation?
1) For those bus devices which have no MMIO( that is to say, indirectIO is
using), I think 'addr' is not needed, but 'size' is mandatory;
I am thinking for our LPC, as there is no cpu address, we should not input
'addr' for the io range register. With 'size' as parameter, we implement a new
io range register function where can assign an unique linux PIO for this
register calling. The output linux PIO can allocate from a sub-range of whole
I/O space of [0, IO_SPACE_LIMIT]. This sub-range is specific for indirectIO, I
want to define a new macro, such as EXTIO_LIMIT, to represent the upper limit of
indirect IO space.
#if defined(CONFIG_PCI) && defined(CONFIG_INDIRECT_PIO)
#define EXTIO_LIMIT PCIBIOS_MIN_IO
#elif defined(CONFIG_INDIRECT_PIO)
#define EXTIO_LIMIT 0x1000
#else
#define EXTIO_LIMIT 0x00
#end
We should do some checkings to ensure EXTIO_LIMIT < IO_SPACE_LIMIT.
Then when someone call pci_register_io_range() or a new function for the linux
PIO register, we can allocate linux PIO from [0, EXTIO_LIMIT) for indirectIO
bus, from [EXTIO_LIMIT, IO_SPACE_LIMIT] for MMIO;
But there are issues confused me yet. For example, how to know the IO size for
the indirectIO bus? You known, there is no 'ranges' property for those buses....
2) For PCI MMIO, I think 'addr' is needed
As for the current pci_register_io_range()/pci_address_to_pio(), I have two doubts:
2.1) If there are multiple PCI host bridges which support I/O transaction, I
wonder whether the first host bridge can access the downstream devices with bus
I/O address in [0, PCIBIOS_MIN_IO)
for the first host bridge, pci_address_to_pio() will return a linux PIO range
start from 0.
But when calling __pci_assign_resource() to allocate the linux PIO for PCI/PCIE
devices/buses which are just children of first host bus, it can not allocate
linux PIO less than PCIBIOS_MIN_IO, which means kernel can not called in/out()
with port less than PCIBIOS_MIN_IO. But we had ioremap [PCI_IOBASE + 0,
PCI_IOBASE + size) to [pci_ioadd, pci_ioadd + size) before.
static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
int resno, resource_size_t size, resource_size_t align)
{
struct resource *res = dev->resource + resno;
resource_size_t min;
int ret;
min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
and in the later function:
static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
resource_size_t size, resource_size_t align,
resource_size_t min, unsigned long type_mask,
resource_size_t (*alignf)(void *,
....
pci_bus_for_each_resource(bus, r, i) {
resource_size_t min_used = min;
....
if (avail.start)
min_used = avail.start;
max = avail.end;
/* Ok, try it out.. */
ret = allocate_resource(r, res, size, min_used, max,
align, alignf, alignf_data);
After allocate_resource(), a IO resource is allocated, but whose 'start' is not
less than min_used.( since avail.start is 0, min_used will keep the 'min'
without change to avail.start; Should be PCIBIOS_MIN_IO).
2.2) Is it possible the return linux PIO isn't page-aligned?
When calling pci_remap_iospace(const struct resource *res, phys_addr_t
phys_addr), if res->start is not page-aligned, it seems that
ioremap_page_range() will meet some issues for duplication iorempa for same
virtual page.
of-course, if we always configure the I/O ranges size as page-aligned, it will
be OK.
I found PowerPC will ensure the 'vaddress' and the 'size' are page-aligned
before ioremap, do we need to improve the current handling in
pci_register_io_range/pci_address_to_pio?
Thanks,
Zhichang
> + if (addr != IO_RANGE_IOEXT &&
> + addr >= range->start &&
> + addr + size <= range->start + size) {
> /* range already registered, bail out */
> goto end_register;
> }
> @@ -3298,6 +3307,7 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> goto end_register;
> }
>
> + range->node = node;
> range->start = addr;
> range->size = size;
>
> @@ -3305,11 +3315,26 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
>
> end_register:
> spin_unlock(&io_range_lock);
> +
> + *port = allocated_size;
> +#else
> + /*
> + * powerpc and microblaze have their own registration,
> + * just look up the value here
> + */
> + *port = pci_address_to_pio(addr);
> #endif
>
> return err;
> }
>
> +#ifdef CONFIG_IOEXT
> +int ioext_register_io_range
> +{
> + return pci_register_io_range(node, IO_RANGE_IOEXT, size, port);
> +}
> +#endif
> +
> phys_addr_t pci_pio_to_address(unsigned long pio)
> {
> phys_addr_t address = (phys_addr_t)OF_BAD_ADDR;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6bd94a803e8f..b7a8fa3da3ca 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1192,7 +1192,8 @@ int __must_check pci_bus_alloc_resource(struct pci_bus *bus,
> void *alignf_data);
>
>
> -int pci_register_io_range(phys_addr_t addr, resource_size_t size);
> +int pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr,
> + resource_size_t size, unsigned long *port);
> unsigned long pci_address_to_pio(phys_addr_t addr);
> phys_addr_t pci_pio_to_address(unsigned long pio);
> int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
>
>
> .
>
^ permalink raw reply
* [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v67M8DrPaf8GzSPEjekgV6cLcXXzO3tVUc9kjUDcM3BE_w@mail.gmail.com>
Hi,
On 24/11/16 04:16, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
>> have arm64 capable cores. Add the generic sunxi config symbol to allow
>> the driver to be selected by arm64 Kconfigs, which don't feature
>> SoC specific MACH_xxxx configs.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>> drivers/dma/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index af63a6b..003c284 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -157,7 +157,7 @@ config DMA_SUN4I
>>
>> config DMA_SUN6I
>> tristate "Allwinner A31 SoCs DMA support"
>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
>
> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
> (And I don't have to add MACH_SUN9I later :) )
Sure, admittedly it was just a quick hack to get things going.
Actually I don't know why we had a *depend* on those MACH_s before. I
think technically it does not depend on a certain SoC (having the
COMPILE_TEST in there hints on that). So what about:
depends on ARCH_SUNXI || COMPILE_TEST
and maybe:
default y if MACH_SUN6I || MACH_SUN8I
Though I see that both multi_v7_defconfig and sunxi_defconfig explicitly
set this, so this wouldn't be needed?
Cheers,
Andre.
^ permalink raw reply
* [PATCH 0/4] net: thunderx: Support for 80xx, RED, PFC e.t.c
From: sunil.kovvuri at gmail.com @ 2016-11-24 9:17 UTC (permalink / raw)
To: linux-arm-kernel
From: Sunil Goutham <sgoutham@cavium.com>
This patch series adds support for SLM modules present on 80xx
silicon, enables ramdom early discard, backpressure generation,
PFC and some ethtool changes to display supported link modes e.t.c.
Sunil Goutham (3):
net: thunderx: 80xx BGX0 configuration changes
net: thunderx: Configure RED and backpressure levels
net: thunderx: Pause frame support
Thanneeru Srinivasulu (1):
net: thunderx: Add ethtool support for supported ports and link modes.
drivers/net/ethernet/cavium/thunder/nic.h | 19 +++++
drivers/net/ethernet/cavium/thunder/nic_main.c | 37 +++++++++
.../net/ethernet/cavium/thunder/nicvf_ethtool.c | 87 +++++++++++++++++++++-
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 7 ++
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 9 ++-
drivers/net/ethernet/cavium/thunder/nicvf_queues.h | 24 ++++--
drivers/net/ethernet/cavium/thunder/q_struct.h | 8 +-
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 74 +++++++++++++++++-
drivers/net/ethernet/cavium/thunder/thunder_bgx.h | 12 +++
9 files changed, 262 insertions(+), 15 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/4] net: thunderx: 80xx BGX0 configuration changes
From: sunil.kovvuri at gmail.com @ 2016-11-24 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479979083-15963-1-git-send-email-sunil.kovvuri@gmail.com>
From: Sunil Goutham <sgoutham@cavium.com>
On 80xx only one lane of DLM0 and DLM1 (of BGX0) can be used
, so even though lmac count may be 2 but LMAC1 should use
serdes lane of DLM1. Since it's not possible to distinguish
80xx from 81xx as PCI devid are same, this patch adds this
config support by replying on what firmware configures the
lmacs with.
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 050e21f..1d6214b 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -970,11 +970,25 @@ static void bgx_set_lmac_config(struct bgx *bgx, u8 idx)
lmac_set_training(bgx, lmac, lmac->lmacid);
lmac_set_lane2sds(bgx, lmac);
- /* Set LMAC type of other lmac on same DLM i.e LMAC 1/3 */
olmac = &bgx->lmac[idx + 1];
- olmac->lmac_type = lmac->lmac_type;
+ /* Check if other LMAC on the same DLM is already configured by
+ * firmware, if so use the same config or else set as same, as
+ * that of LMAC 0/2.
+ * This check is needed as on 80xx only one lane of each of the
+ * DLM of BGX0 is used, so have to rely on firmware for
+ * distingushing 80xx from 81xx.
+ */
+ cmr_cfg = bgx_reg_read(bgx, idx + 1, BGX_CMRX_CFG);
+ lmac_type = (u8)((cmr_cfg >> 8) & 0x07);
+ lane_to_sds = (u8)(cmr_cfg & 0xFF);
+ if ((lmac_type == 0) && (lane_to_sds == 0xE4)) {
+ olmac->lmac_type = lmac->lmac_type;
+ lmac_set_lane2sds(bgx, olmac);
+ } else {
+ olmac->lmac_type = lmac_type;
+ olmac->lane_to_sds = lane_to_sds;
+ }
lmac_set_training(bgx, olmac, olmac->lmacid);
- lmac_set_lane2sds(bgx, olmac);
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH 2/4] net: thunderx: Add ethtool support for supported ports and link modes.
From: sunil.kovvuri at gmail.com @ 2016-11-24 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479979083-15963-1-git-send-email-sunil.kovvuri@gmail.com>
From: Thanneeru Srinivasulu <tsrinivasulu@cavium.com>
Signed-off-by: Thanneeru Srinivasulu <tsrinivasulu@cavium.com>
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 2 ++
drivers/net/ethernet/cavium/thunder/nic_main.c | 1 +
.../net/ethernet/cavium/thunder/nicvf_ethtool.c | 36 ++++++++++++++++++++--
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 1 +
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 1 +
5 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 86bd93c..be8404a 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -292,6 +292,7 @@ struct nicvf {
u8 node;
u8 cpi_alg;
bool link_up;
+ u8 mac_type;
u8 duplex;
u32 speed;
bool tns_mode;
@@ -446,6 +447,7 @@ struct bgx_stats_msg {
/* Physical interface link status */
struct bgx_link_status {
u8 msg;
+ u8 mac_type;
u8 link_up;
u8 duplex;
u32 speed;
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 6677b96..b87d416 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -1258,6 +1258,7 @@ static void nic_poll_for_link(struct work_struct *work)
mbx.link_status.link_up = link.link_up;
mbx.link_status.duplex = link.duplex;
mbx.link_status.speed = link.speed;
+ mbx.link_status.mac_type = link.mac_type;
nic_send_msg_to_vf(nic, vf, &mbx);
}
}
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
index 432bf6b..d4d76a7 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
@@ -130,12 +130,42 @@ static int nicvf_get_settings(struct net_device *netdev,
return 0;
}
- if (nic->speed <= 1000) {
- cmd->port = PORT_MII;
+ switch (nic->speed) {
+ case SPEED_1000:
+ cmd->port = PORT_MII | PORT_TP;
cmd->autoneg = AUTONEG_ENABLE;
- } else {
+ cmd->supported |= SUPPORTED_MII | SUPPORTED_TP;
+ cmd->supported |= SUPPORTED_1000baseT_Full |
+ SUPPORTED_1000baseT_Half |
+ SUPPORTED_100baseT_Full |
+ SUPPORTED_100baseT_Half |
+ SUPPORTED_10baseT_Full |
+ SUPPORTED_10baseT_Half;
+ cmd->supported |= SUPPORTED_Autoneg;
+ cmd->advertising |= ADVERTISED_1000baseT_Full |
+ ADVERTISED_1000baseT_Half |
+ ADVERTISED_100baseT_Full |
+ ADVERTISED_100baseT_Half |
+ ADVERTISED_10baseT_Full |
+ ADVERTISED_10baseT_Half;
+ break;
+ case SPEED_10000:
+ if (nic->mac_type == BGX_MODE_RXAUI) {
+ cmd->port = PORT_TP;
+ cmd->supported |= SUPPORTED_TP;
+ } else {
+ cmd->port = PORT_FIBRE;
+ cmd->supported |= SUPPORTED_FIBRE;
+ }
+ cmd->autoneg = AUTONEG_DISABLE;
+ cmd->supported |= SUPPORTED_10000baseT_Full;
+ break;
+ case SPEED_40000:
cmd->port = PORT_FIBRE;
cmd->autoneg = AUTONEG_DISABLE;
+ cmd->supported |= SUPPORTED_FIBRE;
+ cmd->supported |= SUPPORTED_40000baseCR4_Full;
+ break;
}
cmd->duplex = nic->duplex;
ethtool_cmd_speed_set(cmd, nic->speed);
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 7c2c373..c6c2303 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -221,6 +221,7 @@ static void nicvf_handle_mbx_intr(struct nicvf *nic)
nic->link_up = mbx.link_status.link_up;
nic->duplex = mbx.link_status.duplex;
nic->speed = mbx.link_status.speed;
+ nic->mac_type = mbx.link_status.mac_type;
if (nic->link_up) {
netdev_info(nic->netdev, "%s: Link is Up %d Mbps %s\n",
nic->netdev->name, nic->speed,
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 1d6214b..29c727f 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -161,6 +161,7 @@ void bgx_get_lmac_link_state(int node, int bgx_idx, int lmacid, void *status)
return;
lmac = &bgx->lmac[lmacid];
+ link->mac_type = lmac->lmac_type;
link->link_up = lmac->link_up;
link->duplex = lmac->last_duplex;
link->speed = lmac->last_speed;
--
2.7.4
^ permalink raw reply related
* [PATCH 3/4] net: thunderx: Configure RED and backpressure levels
From: sunil.kovvuri at gmail.com @ 2016-11-24 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479979083-15963-1-git-send-email-sunil.kovvuri@gmail.com>
From: Sunil Goutham <sgoutham@cavium.com>
This patch enables moving average calculation of Rx pkt's resources
and configures RED and backpressure levels for both CQ and RBDR.
Also initialize SQ's CQ_LIMIT properly.
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/nic_main.c | 9 ++++++++
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 9 ++++++--
drivers/net/ethernet/cavium/thunder/nicvf_queues.h | 24 +++++++++++++++++-----
drivers/net/ethernet/cavium/thunder/q_struct.h | 8 ++++++--
4 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index b87d416..17490ec 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -809,6 +809,15 @@ static int nic_config_loopback(struct nicpf *nic, struct set_loopback *lbk)
bgx_lmac_internal_loopback(nic->node, bgx_idx, lmac_idx, lbk->enable);
+ /* Enable moving average calculation.
+ * Keep the LVL/AVG delay to HW enforced minimum so that, not too many
+ * packets sneek in between average calculations.
+ */
+ nic_reg_write(nic, NIC_PF_CQ_AVG_CFG,
+ (BIT_ULL(20) | 0x2ull << 14 | 0x1));
+ nic_reg_write(nic, NIC_PF_RRM_AVG_CFG,
+ (BIT_ULL(20) | 0x3ull << 14 | 0x1));
+
return 0;
}
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 747ef08..7b336cd 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -544,14 +544,18 @@ static void nicvf_rcv_queue_config(struct nicvf *nic, struct queue_set *qs,
nicvf_send_msg_to_pf(nic, &mbx);
mbx.rq.msg = NIC_MBOX_MSG_RQ_BP_CFG;
- mbx.rq.cfg = (1ULL << 63) | (1ULL << 62) | (qs->vnic_id << 0);
+ mbx.rq.cfg = BIT_ULL(63) | BIT_ULL(62) |
+ (RQ_PASS_RBDR_LVL << 16) | (RQ_PASS_CQ_LVL << 8) |
+ (qs->vnic_id << 0);
nicvf_send_msg_to_pf(nic, &mbx);
/* RQ drop config
* Enable CQ drop to reserve sufficient CQEs for all tx packets
*/
mbx.rq.msg = NIC_MBOX_MSG_RQ_DROP_CFG;
- mbx.rq.cfg = (1ULL << 62) | (RQ_CQ_DROP << 8);
+ mbx.rq.cfg = BIT_ULL(63) | BIT_ULL(62) |
+ (RQ_PASS_RBDR_LVL << 40) | (RQ_DROP_RBDR_LVL << 32) |
+ (RQ_PASS_CQ_LVL << 16) | (RQ_DROP_CQ_LVL << 8);
nicvf_send_msg_to_pf(nic, &mbx);
if (!nic->sqs_mode && (qidx == 0)) {
@@ -650,6 +654,7 @@ static void nicvf_snd_queue_config(struct nicvf *nic, struct queue_set *qs,
sq_cfg.ldwb = 0;
sq_cfg.qsize = SND_QSIZE;
sq_cfg.tstmp_bgx_intf = 0;
+ sq_cfg.cq_limit = 0;
nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_CFG, qidx, *(u64 *)&sq_cfg);
/* Set threshold value for interrupt generation */
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
index 2e3c940..20511f2 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
@@ -85,12 +85,26 @@
#define MAX_CQES_FOR_TX ((SND_QUEUE_LEN / MIN_SQ_DESC_PER_PKT_XMIT) * \
MAX_CQE_PER_PKT_XMIT)
-/* Calculate number of CQEs to reserve for all SQEs.
- * Its 1/256th level of CQ size.
- * '+ 1' to account for pipelining
+
+/* RED and Backpressure levels of CQ for pkt reception
+ * For CQ, level is a measure of emptiness i.e 0x0 means full
+ * eg: For CQ of size 4K, and for pass/drop levels of 128/96
+ * HW accepts pkt if unused CQE >= 2048
+ * RED accepts pkt if unused CQE < 2048 & >= 1536
+ * DROPs pkts if unused CQE < 1536
+ */
+#define RQ_PASS_CQ_LVL 128ULL
+#define RQ_DROP_CQ_LVL 96ULL
+
+/* RED and Backpressure levels of RBDR for pkt reception
+ * For RBDR, level is a measure of fullness i.e 0x0 means empty
+ * eg: For RBDR of size 8K, and for pass/drop levels of 4/0
+ * HW accepts pkt if unused RBs >= 256
+ * RED accepts pkt if unused RBs < 256 & >= 0
+ * DROPs pkts if unused RBs < 0
*/
-#define RQ_CQ_DROP ((256 / (CMP_QUEUE_LEN / \
- (CMP_QUEUE_LEN - MAX_CQES_FOR_TX))) + 1)
+#define RQ_PASS_RBDR_LVL 8ULL
+#define RQ_DROP_RBDR_LVL 0ULL
/* Descriptor size in bytes */
#define SND_QUEUE_DESC_SIZE 16
diff --git a/drivers/net/ethernet/cavium/thunder/q_struct.h b/drivers/net/ethernet/cavium/thunder/q_struct.h
index 9e6d987..f363472 100644
--- a/drivers/net/ethernet/cavium/thunder/q_struct.h
+++ b/drivers/net/ethernet/cavium/thunder/q_struct.h
@@ -624,7 +624,9 @@ struct cq_cfg {
struct sq_cfg {
#if defined(__BIG_ENDIAN_BITFIELD)
- u64 reserved_20_63:44;
+ u64 reserved_32_63:32;
+ u64 cq_limit:8;
+ u64 reserved_20_23:4;
u64 ena:1;
u64 reserved_18_18:1;
u64 reset:1;
@@ -642,7 +644,9 @@ struct sq_cfg {
u64 reset:1;
u64 reserved_18_18:1;
u64 ena:1;
- u64 reserved_20_63:44;
+ u64 reserved_20_23:4;
+ u64 cq_limit:8;
+ u64 reserved_32_63:32;
#endif
};
--
2.7.4
^ permalink raw reply related
* [PATCH 4/4] net: thunderx: Pause frame support
From: sunil.kovvuri at gmail.com @ 2016-11-24 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479979083-15963-1-git-send-email-sunil.kovvuri@gmail.com>
From: Sunil Goutham <sgoutham@cavium.com>
Enable pause frames on both Rx and Tx side, configure pause
interval e.t.c. Also support for enable/disable pause frames
on Rx/Tx via ethtool has been added.
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 17 +++++++
drivers/net/ethernet/cavium/thunder/nic_main.c | 27 +++++++++++
.../net/ethernet/cavium/thunder/nicvf_ethtool.c | 51 +++++++++++++++++++++
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 6 +++
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 53 ++++++++++++++++++++++
drivers/net/ethernet/cavium/thunder/thunder_bgx.h | 12 +++++
6 files changed, 166 insertions(+)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index be8404a..e739c71 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -149,6 +149,12 @@ struct nicvf_rss_info {
u64 key[RSS_HASH_KEY_SIZE];
} ____cacheline_aligned_in_smp;
+struct nicvf_pfc {
+ u8 autoneg;
+ u8 fc_rx;
+ u8 fc_tx;
+};
+
enum rx_stats_reg_offset {
RX_OCTS = 0x0,
RX_UCAST = 0x1,
@@ -298,6 +304,7 @@ struct nicvf {
bool tns_mode;
bool loopback_supported;
struct nicvf_rss_info rss_info;
+ struct nicvf_pfc pfc;
struct tasklet_struct qs_err_task;
struct work_struct reset_task;
@@ -358,6 +365,7 @@ struct nicvf {
#define NIC_MBOX_MSG_SNICVF_PTR 0x15 /* Send sqet nicvf ptr to PVF */
#define NIC_MBOX_MSG_LOOPBACK 0x16 /* Set interface in loopback */
#define NIC_MBOX_MSG_RESET_STAT_COUNTER 0x17 /* Reset statistics counters */
+#define NIC_MBOX_MSG_PFC 0x18 /* Pause frame control */
#define NIC_MBOX_MSG_CFG_DONE 0xF0 /* VF configuration done */
#define NIC_MBOX_MSG_SHUTDOWN 0xF1 /* VF is being shutdown */
@@ -500,6 +508,14 @@ struct reset_stat_cfg {
u16 sq_stat_mask;
};
+struct pfc {
+ u8 msg;
+ u8 get; /* Get or set PFC settings */
+ u8 autoneg;
+ u8 fc_rx;
+ u8 fc_tx;
+};
+
/* 128 bit shared memory between PF and each VF */
union nic_mbx {
struct { u8 msg; } msg;
@@ -518,6 +534,7 @@ union nic_mbx {
struct nicvf_ptr nicvf;
struct set_loopback lbk;
struct reset_stat_cfg reset_stat;
+ struct pfc pfc;
};
#define NIC_NODE_ID_MASK 0x03
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 17490ec..767234e 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -898,6 +898,30 @@ static void nic_enable_vf(struct nicpf *nic, int vf, bool enable)
bgx_lmac_rx_tx_enable(nic->node, bgx, lmac, enable);
}
+static void nic_pause_frame(struct nicpf *nic, int vf, struct pfc *cfg)
+{
+ int bgx, lmac;
+ struct pfc pfc;
+ union nic_mbx mbx = {};
+
+ if (vf >= nic->num_vf_en)
+ return;
+ bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+ lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+
+ if (cfg->get) {
+ bgx_lmac_get_pfc(nic->node, bgx, lmac, &pfc);
+ mbx.pfc.msg = NIC_MBOX_MSG_PFC;
+ mbx.pfc.autoneg = pfc.autoneg;
+ mbx.pfc.fc_rx = pfc.fc_rx;
+ mbx.pfc.fc_tx = pfc.fc_tx;
+ nic_send_msg_to_vf(nic, vf, &mbx);
+ } else {
+ bgx_lmac_set_pfc(nic->node, bgx, lmac, cfg);
+ nic_mbx_send_ack(nic, vf);
+ }
+}
+
/* Interrupt handler to handle mailbox messages from VFs */
static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
{
@@ -1037,6 +1061,9 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
case NIC_MBOX_MSG_RESET_STAT_COUNTER:
ret = nic_reset_stat_counters(nic, vf, &mbx.reset_stat);
break;
+ case NIC_MBOX_MSG_PFC:
+ nic_pause_frame(nic, vf, &mbx.pfc);
+ goto unlock;
default:
dev_err(&nic->pdev->dev,
"Invalid msg from VF%d, msg 0x%x\n", vf, mbx.msg.msg);
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
index d4d76a7..b048241 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
@@ -720,6 +720,55 @@ static int nicvf_set_channels(struct net_device *dev,
return err;
}
+static void nicvf_get_pauseparam(struct net_device *dev,
+ struct ethtool_pauseparam *pause)
+{
+ struct nicvf *nic = netdev_priv(dev);
+ union nic_mbx mbx = {};
+
+ /* Supported only for 10G/40G interfaces */
+ if ((nic->mac_type == BGX_MODE_SGMII) ||
+ (nic->mac_type == BGX_MODE_QSGMII) ||
+ (nic->mac_type == BGX_MODE_RGMII))
+ return;
+
+ mbx.pfc.msg = NIC_MBOX_MSG_PFC;
+ mbx.pfc.get = 1;
+ if (!nicvf_send_msg_to_pf(nic, &mbx)) {
+ pause->autoneg = nic->pfc.autoneg;
+ pause->rx_pause = nic->pfc.fc_rx;
+ pause->tx_pause = nic->pfc.fc_tx;
+ }
+}
+
+static int nicvf_set_pauseparam(struct net_device *dev,
+ struct ethtool_pauseparam *pause)
+{
+ struct nicvf *nic = netdev_priv(dev);
+ union nic_mbx mbx = {};
+
+ /* Supported only for 10G/40G interfaces */
+ if ((nic->mac_type == BGX_MODE_SGMII) ||
+ (nic->mac_type == BGX_MODE_QSGMII) ||
+ (nic->mac_type == BGX_MODE_RGMII))
+ return -EOPNOTSUPP;
+
+ if (pause->autoneg)
+ return -EOPNOTSUPP;
+
+ mbx.pfc.msg = NIC_MBOX_MSG_PFC;
+ mbx.pfc.get = 0;
+ mbx.pfc.fc_rx = pause->rx_pause;
+ mbx.pfc.fc_tx = pause->tx_pause;
+ if (nicvf_send_msg_to_pf(nic, &mbx))
+ return -EAGAIN;
+
+ nic->pfc.fc_rx = pause->rx_pause;
+ nic->pfc.fc_tx = pause->tx_pause;
+
+ return 0;
+}
+
static const struct ethtool_ops nicvf_ethtool_ops = {
.get_settings = nicvf_get_settings,
.get_link = nicvf_get_link,
@@ -741,6 +790,8 @@ static const struct ethtool_ops nicvf_ethtool_ops = {
.set_rxfh = nicvf_set_rxfh,
.get_channels = nicvf_get_channels,
.set_channels = nicvf_set_channels,
+ .get_pauseparam = nicvf_get_pauseparam,
+ .set_pauseparam = nicvf_set_pauseparam,
.get_ts_info = ethtool_op_get_ts_info,
};
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index c6c2303..1eacec8 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -256,6 +256,12 @@ static void nicvf_handle_mbx_intr(struct nicvf *nic)
nic->pnicvf = (struct nicvf *)mbx.nicvf.nicvf;
nic->pf_acked = true;
break;
+ case NIC_MBOX_MSG_PFC:
+ nic->pfc.autoneg = mbx.pfc.autoneg;
+ nic->pfc.fc_rx = mbx.pfc.fc_rx;
+ nic->pfc.fc_tx = mbx.pfc.fc_tx;
+ nic->pf_acked = true;
+ break;
default:
netdev_err(nic->netdev,
"Invalid message from PF, msg 0x%x\n", mbx.msg.msg);
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 29c727f..9211c75 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -212,6 +212,47 @@ void bgx_lmac_rx_tx_enable(int node, int bgx_idx, int lmacid, bool enable)
}
EXPORT_SYMBOL(bgx_lmac_rx_tx_enable);
+void bgx_lmac_get_pfc(int node, int bgx_idx, int lmacid, void *pause)
+{
+ struct pfc *pfc = (struct pfc *)pause;
+ struct bgx *bgx = bgx_vnic[(node * MAX_BGX_PER_CN88XX) + bgx_idx];
+ struct lmac *lmac;
+ u64 cfg;
+
+ if (!bgx)
+ return;
+ lmac = &bgx->lmac[lmacid];
+ if (lmac->is_sgmii)
+ return;
+
+ cfg = bgx_reg_read(bgx, lmacid, BGX_SMUX_CBFC_CTL);
+ pfc->fc_rx = cfg & RX_EN;
+ pfc->fc_tx = cfg & TX_EN;
+ pfc->autoneg = 0;
+}
+EXPORT_SYMBOL(bgx_lmac_get_pfc);
+
+void bgx_lmac_set_pfc(int node, int bgx_idx, int lmacid, void *pause)
+{
+ struct pfc *pfc = (struct pfc *)pause;
+ struct bgx *bgx = bgx_vnic[(node * MAX_BGX_PER_CN88XX) + bgx_idx];
+ struct lmac *lmac;
+ u64 cfg;
+
+ if (!bgx)
+ return;
+ lmac = &bgx->lmac[lmacid];
+ if (lmac->is_sgmii)
+ return;
+
+ cfg = bgx_reg_read(bgx, lmacid, BGX_SMUX_CBFC_CTL);
+ cfg &= ~(RX_EN | TX_EN);
+ cfg |= (pfc->fc_rx ? RX_EN : 0x00);
+ cfg |= (pfc->fc_tx ? TX_EN : 0x00);
+ bgx_reg_write(bgx, lmacid, BGX_SMUX_CBFC_CTL, cfg);
+}
+EXPORT_SYMBOL(bgx_lmac_set_pfc);
+
static void bgx_sgmii_change_link_state(struct lmac *lmac)
{
struct bgx *bgx = lmac->bgx;
@@ -525,6 +566,18 @@ static int bgx_lmac_xaui_init(struct bgx *bgx, struct lmac *lmac)
cfg |= SMU_TX_CTL_DIC_EN;
bgx_reg_write(bgx, lmacid, BGX_SMUX_TX_CTL, cfg);
+ /* Enable receive and transmission of pause frames */
+ bgx_reg_write(bgx, lmacid, BGX_SMUX_CBFC_CTL, ((0xffffULL << 32) |
+ BCK_EN | DRP_EN | TX_EN | RX_EN));
+ /* Configure pause time and interval */
+ bgx_reg_write(bgx, lmacid,
+ BGX_SMUX_TX_PAUSE_PKT_TIME, DEFAULT_PAUSE_TIME);
+ cfg = bgx_reg_read(bgx, lmacid, BGX_SMUX_TX_PAUSE_PKT_INTERVAL);
+ cfg &= ~0xFFFFull;
+ bgx_reg_write(bgx, lmacid, BGX_SMUX_TX_PAUSE_PKT_INTERVAL,
+ cfg | (DEFAULT_PAUSE_TIME - 0x1000));
+ bgx_reg_write(bgx, lmacid, BGX_SMUX_TX_PAUSE_ZERO, 0x01);
+
/* take lmac_count into account */
bgx_reg_modify(bgx, lmacid, BGX_SMUX_TX_THRESH, (0x100 - 1));
/* max packet size */
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
index 01cc7c8..c18ebfe 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
@@ -27,6 +27,7 @@
#define MAX_BGX_CHANS_PER_LMAC 16
#define MAX_DMAC_PER_LMAC 8
#define MAX_FRAME_SIZE 9216
+#define DEFAULT_PAUSE_TIME 0xFFFF
#define BGX_ID_MASK 0x3
@@ -126,7 +127,10 @@
#define SMU_RX_CTL_STATUS (3ull << 0)
#define BGX_SMUX_TX_APPEND 0x20100
#define SMU_TX_APPEND_FCS_D BIT_ULL(2)
+#define BGX_SMUX_TX_PAUSE_PKT_TIME 0x20110
#define BGX_SMUX_TX_MIN_PKT 0x20118
+#define BGX_SMUX_TX_PAUSE_PKT_INTERVAL 0x20120
+#define BGX_SMUX_TX_PAUSE_ZERO 0x20138
#define BGX_SMUX_TX_INT 0x20140
#define BGX_SMUX_TX_CTL 0x20178
#define SMU_TX_CTL_DIC_EN BIT_ULL(0)
@@ -136,6 +140,11 @@
#define BGX_SMUX_CTL 0x20200
#define SMU_CTL_RX_IDLE BIT_ULL(0)
#define SMU_CTL_TX_IDLE BIT_ULL(1)
+#define BGX_SMUX_CBFC_CTL 0x20218
+#define RX_EN BIT_ULL(0)
+#define TX_EN BIT_ULL(1)
+#define BCK_EN BIT_ULL(2)
+#define DRP_EN BIT_ULL(3)
#define BGX_GMP_PCS_MRX_CTL 0x30000
#define PCS_MRX_CTL_RST_AN BIT_ULL(9)
@@ -207,6 +216,9 @@ void bgx_set_lmac_mac(int node, int bgx_idx, int lmacid, const u8 *mac);
void bgx_get_lmac_link_state(int node, int bgx_idx, int lmacid, void *status);
void bgx_lmac_internal_loopback(int node, int bgx_idx,
int lmac_idx, bool enable);
+void bgx_lmac_get_pfc(int node, int bgx_idx, int lmacid, void *pause);
+void bgx_lmac_set_pfc(int node, int bgx_idx, int lmacid, void *pause);
+
void xcv_init_hw(void);
void xcv_setup_link(bool link_up, int link_speed);
--
2.7.4
^ permalink raw reply related
* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Gregory CLEMENT @ 2016-11-24 9:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4031579.CBE32NHUoW@wuerfel>
Hi Arnd,
On jeu., nov. 24 2016, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
>> > You also mentioned other bindings using child nodes, but for this one
>> > we have one controller with only one set of register with multiple slots
>> > (Atmel is an example). Here each slot have it own set of register.
>> >
>> > Actually giving the fact that each slot is controlled by a different set
>> > of register I wonder why the hardware can't also deduce the slot number
>> > from the address register. For me it looks like an hardware bug but we
>> > have to deal with it.
>> >
>> > Do you still think we needchild node here?
>>
>> Using child-nodes for slots like what's done in the atmel case, is
>> currently broken. I would recommend to avoid using child-nodes for
>> slots, if possible.
>>
>> To give you some more background, currently the mmc core treats child
>> nodes as embedded non-removable cards or SDIO funcs. However, we can
>> change to make child-nodes also allowed to describe slots, but it
>> requires a specific compatible for "slots" and of course then we also
>> need to update the DT parsing of the child-nodes in the mmc core.
>>
>> Documentation/devicetree/bindings/mmc/mmc.txt
>> Documentation/devicetree/bindings/mmc/mmc-card.txt
>
> I don't see anything wrong with having child nodes for the slots
> even with the current binding, under one condition:
>
> The mmc.txt binding above must refer only to the child node, while
> the parent node conceptually becomes a plain bus or MFD that
> happens to encapsulate multiple MMC host controllers, and possibly
> provides some shared registers to them.
I don't have an option for mmc in general, but using child node do not
fit at all the xenon controller.
For this controller each slot has its own set of register, so there is
no common ressource to share so no advantage to use it. Using child node
in our case will just make the code more complex for no benefit.
Gregory
>
> Arnd
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ 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