Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/2] PCI: kirin: Add MSI support
From: Andy Shevchenko @ 2018-05-13 23:16 UTC (permalink / raw)
  To: Yao Chen
  Cc: songxiaowei, wangbinghui, Lorenzo Pieralisi, Bjorn Helgaas,
	xuwei (O), Rob Herring, Mark Rutland, Catalin Marinas,
	Will Deacon, linux-pci, Linux Kernel Mailing List,
	linux-arm Mailing List, devicetree, dimitrysh, guodong.xu,
	suzhuangluan, kongfei
In-Reply-To: <1526030149-23985-2-git-send-email-chenyao11@huawei.com>

On Fri, May 11, 2018 at 12:15 PM, Yao Chen <chenyao11@huawei.com> wrote:
> Add support for MSI.


> +       int ret;
> +
> +       if (IS_ENABLED(CONFIG_PCI_MSI)) {

> +               pci->pp.msi_irq = platform_get_irq(pdev, 0);
> +               if (pci->pp.msi_irq < 0) {
> +                       dev_err(&pdev->dev, "failed to get MSI IRQ (%d)\n",
> +                               pci->pp.msi_irq);

> +                       return -ENODEV;

Why shadowing actual error code?

> +               }
> +               ret = devm_request_irq(&pdev->dev, pci->pp.msi_irq,
> +                                      kirin_pcie_msi_irq_handler,
> +                                      IRQF_SHARED | IRQF_NO_THREAD,
> +                                      "kirin_pcie_msi", &pci->pp);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "failed to request MSI IRQ %d\n",
> +                               pci->pp.msi_irq);
> +                       return ret;
> +               }

It would be easy to read and maintain if this would be a separate function.

> +       }



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 6/8] serial: Add Tegra Combined UART driver
From: Andy Shevchenko @ 2018-05-13 22:20 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: Mikko Perttunen, Rob Herring, Mark Rutland, Jassi Brar,
	Greg Kroah-Hartman, Thierry Reding, Jon Hunter, araza, devicetree,
	open list:SERIAL DRIVERS, linux-tegra, linux-arm Mailing List,
	Linux Kernel Mailing List
In-Reply-To: <662ce972-56a9-9daa-d332-faa08b89c05a@kapsi.fi>

On Sun, May 13, 2018 at 9:04 PM, Mikko Perttunen <cyndis@kapsi.fi> wrote:
> On 05/13/2018 05:16 PM, Andy Shevchenko wrote:
>>
>> On Tue, May 8, 2018 at 2:44 PM, Mikko Perttunen <mperttunen@nvidia.com>
>> wrote:
>>>
>>> The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
>>> multiplexing multiple "virtual UARTs" into a single hardware serial
>>> port. The TCU is the primary serial port on Tegra194 devices.
>>>
>>> Add a TCU driver utilizing the mailbox framework, as the used mailboxes
>>> are part of Tegra HSP blocks that are already controlled by the Tegra
>>> HSP mailbox driver.

>>> +static void tegra_tcu_uart_set_mctrl(struct uart_port *port, unsigned
>>> int mctrl)
>>> +{
>>
>>
>>> +       (void)port;
>>> +       (void)mctrl;
>>
>>
>> Huh?
>
>
> The serial core calls these callbacks without checking if they are set. They
> don't make sense for this driver so they are stubbed out.

My question why do you need these ugly lines? I'm pretty sure no other
driver with stubs using such style.

>>> +}

>>> +               if (written == 3) {
>>> +                       value |= 3 << 24;
>>> +                       value |= BIT(26);
>>> +                       mbox_send_message(tcu->tx, &value);
>>
>>
>>> +               }
>>
>>
>> (1)
>>
>>> +       }
>>> +
>>> +       if (written) {
>>> +               value |= written << 24;
>>> +               value |= BIT(26);
>>> +               mbox_send_message(tcu->tx, &value);
>>> +       }
>>
>>
>> (2)
>>
>> These are code duplications.
>
>
> Indeed - the length of the duplicated code is so short, and the instances
> are so close to each other, that I don't find it necessary (or clearer) to
> have an extra function.

It makes sense. Consider to refactor other way w/o duplication then.

>>> +static void tegra_tcu_uart_set_termios(struct uart_port *port,
>>> +                                      struct ktermios *new,
>>> +                                      struct ktermios *old)
>>> +{
>>> +       (void)port;
>>> +       (void)new;
>>> +       (void)old;
>>> +}
>>
>>
>> Remove those unused stub contents.
>
>
> Sure. I had these here so that we don't get unused parameter warnings, but I
> can just as well remove the parameter names.

What warnings? How did you get them? We have them disabled as far as I
know even with W=1.

>
>>
>>> +       return uart_set_options(&tegra_tcu_uart_port, cons,
>>> +                               115200, 'n', 8, 'n');
>>
>>
>> Can't it be one line?
>
>
> It would be a total of 81 characters in length on one line, so no.

So, yes. 1 character doesn't prevent us make the readability better.
Please, put to one line.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [RFC PATCH] driver core: make deferring probe forever optional
From: Linus Walleij @ 2018-05-13 22:01 UTC (permalink / raw)
  To: Alexander Graf, Sören Brinkmann
  Cc: Bjorn Andersson, Rob Herring, linux-kernel@vger.kernel.org,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Grant Likely, Mark Brown, Stephen Boyd,
	boot-architecture, Linux ARM
In-Reply-To: <91dad272-e07d-2ab6-2afa-538294e9cefa@suse.de>

On Wed, May 9, 2018 at 11:44 AM, Alexander Graf <agraf@suse.de> wrote:
> On 05/07/2018 08:31 PM, Bjorn Andersson wrote:

>> Can you please name platform that has enough support for Alexander to
>> care about backwards and forwards compatibility but lacks a pinctrl
>> driver.
>
> ZynqMP is one example that immediately comes to my mind. I'm sure there are
> others too.

Why isn't that using drivers/pinctrl/pinctrl-zynq.c?

How is it so very different from (old) Zynq as it is already using
the same GPIO driver?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH/RFT v3 1/3] thermal: rcar_thermal: add r8a77995 support
From: Yoshihiro Kaneko @ 2018-05-13 21:11 UTC (permalink / raw)
  To: Simon Horman
  Cc: Eduardo Valentin, Rob Herring, Zhang Rui, devicetree, linux-pm,
	linux-renesas-soc
In-Reply-To: <20180509181110.56ylpjqtefwnau3r@verge.net.au>

[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]

Hi Simon-san,

2018年5月10日(木) 3:11 Simon Horman <horms@verge.net.au>:

> On Tue, Apr 03, 2018 at 09:43:03PM +0900, Yoshihiro Kaneko wrote:
> > Add support for R-Car D3 (r8a77995) thermal sensor.
> >
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > ---
> >  drivers/thermal/rcar_thermal.c | 154
> ++++++++++++++++++++++++++++++++---------
> >  1 file changed, 122 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/thermal/rcar_thermal.c
> b/drivers/thermal/rcar_thermal.c
> > index 73e5fee..5ec47a9 100644
> > --- a/drivers/thermal/rcar_thermal.c
> > +++ b/drivers/thermal/rcar_thermal.c
> > @@ -58,10 +58,43 @@ struct rcar_thermal_common {
> >       spinlock_t lock;
> >  };
> >
> > +struct rcar_thermal_chip {
> > +     unsigned int use_of_thermal : 1;
> > +     unsigned int has_filonoff : 1;
> > +     unsigned int irq_per_ch : 1;
> > +     unsigned int needs_suspend_resume : 1;
> > +     unsigned int nirqs;
> > +};
> > +
> > +static const struct rcar_thermal_chip rcar_thermal = {
> > +     .use_of_thermal = 0,
> > +     .has_filonoff = 1,
> > +     .irq_per_ch = 0,
> > +     .needs_suspend_resume = 0,
> > +     .nirqs = 1,
> > +};
> > +
> > +static const struct rcar_thermal_chip rcar_gen2_thermal = {
> > +     .use_of_thermal = 1,
> > +     .has_filonoff = 1,
> > +     .irq_per_ch = 0,
> > +     .needs_suspend_resume = 0,
> > +     .nirqs = 1,
> > +};
> > +
> > +static const struct rcar_thermal_chip rcar_gen3_thermal = {
> > +     .use_of_thermal = 1,
> > +     .has_filonoff = 0,
> > +     .irq_per_ch = 1,
> > +     .needs_suspend_resume = 1,
> > +     .nirqs = 2,
> > +};
>
> The binding and dts patches in this series describe 3 interrupts
> for R-Car D3. But the above specifies two. Am I missing something obvious?


R-Car D3 has 3 interrupts, but this driver uses only 2 interrupts to detect
a temperature change, rise or fall.

[-- Attachment #2: Type: text/html, Size: 2738 bytes --]

^ permalink raw reply

* Re: [PATCH/RFT v3 2/3] dt-bindings: thermal: rcar-thermal: add R8A77995 support
From: Yoshihiro Kaneko @ 2018-05-13 20:55 UTC (permalink / raw)
  To: Simon Horman
  Cc: Eduardo Valentin, Rob Herring, Zhang Rui, devicetree, linux-pm,
	linux-renesas-soc
In-Reply-To: <20180509180950.b35wrl35sxunjmpg@verge.net.au>

[-- Attachment #1: Type: text/plain, Size: 2595 bytes --]

Hi Simon-san,

2018年5月10日(木) 3:10 Simon Horman <horms@verge.net.au>:

> On Mon, Apr 09, 2018 at 04:21:29PM -0500, Rob Herring wrote:
> > On Tue, Apr 03, 2018 at 09:43:04PM +0900, Yoshihiro Kaneko wrote:
> > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  Documentation/devicetree/bindings/thermal/rcar-thermal.txt | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> > > index 349e635..5ab5fcd 100644
> > > --- a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> > > +++ b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> > > @@ -3,7 +3,8 @@
> > >  Required properties:
> > >  - compatible               : "renesas,thermal-<soctype>",
> > >                        "renesas,rcar-gen2-thermal" (with thermal-zone)
> or
> > > -                      "renesas,rcar-thermal" (without thermal-zone)
> as fallback.
> > > +                      "renesas,rcar-thermal" (without thermal-zone) as
> > > +                           fallback except R-Car D3.
> > >                       Examples with soctypes are:
> > >                         - "renesas,thermal-r8a73a4" (R-Mobile APE6)
> > >                         - "renesas,thermal-r8a7743" (RZ/G1M)
> > > @@ -12,13 +13,15 @@ Required properties:
> > >                         - "renesas,thermal-r8a7791" (R-Car M2-W)
> > >                         - "renesas,thermal-r8a7792" (R-Car V2H)
> > >                         - "renesas,thermal-r8a7793" (R-Car M2-N)
> > > +                       - "renesas,thermal-r8a77995" (R-Car D3)
> > >  - reg                      : Address range of the thermal registers.
> > >                       The 1st reg will be recognized as common register
> > >                       if it has "interrupts".
> > >
> > >  Option properties:
> > >
> > > -- interrupts               : use interrupt
> > > +- interrupts               : use interrupt.
> > > +                          Should contain 3 interrupts for R-Car D3.
> >
> > And how many for other chips?
>
> How about this?
>
>                                 If present should contain 3 interrupts for
>                                 R-Car D3 or 1 interrupt otherwise.


Thanks!
I will update the patch with your suggestion.


>
> > >
> > >  Example (non interrupt support):
> > >
> > > --
> > > 1.9.1
> > >
> >
>

[-- Attachment #2: Type: text/html, Size: 3900 bytes --]

^ permalink raw reply

* Re: [PATCH v1 11/13] dt-bindings: power: add PX30 SoCs header for power-domain
From: Heiko Stübner @ 2018-05-13 20:53 UTC (permalink / raw)
  To: Tao Huang
  Cc: Elaine Zhang, mark.rutland, devicetree, ulf.hansson, khilman, xxx,
	linux-pm, rjw, linux-kernel, xf, linux-rockchip, robh+dt,
	Finley Xiao, linux-arm-kernel, wxt
In-Reply-To: <9c9606f0-5c97-1a6e-a3c5-f6380c16fea4@rock-chips.com>

Hi,

Am Sonntag, 13. Mai 2018, 17:18:12 CEST schrieb Tao Huang:
> Hi Heiko:
> 
> On 2018年05月12日 06:11, Heiko Stuebner wrote:
> > Here I have a naming question. When looking at the vendor kernel
> > it looks like the px30 is largely related to the rk3326.
> > (rk3326.dtsi includeing the px30.dtsi)
> > 
> > What is the reason for basing the naming on the px30 this time? And could
> > we possibly keep to rkXXXX names for the basic things in the kernel, thus
> > keeping the pxXX as second name, like with the other px-variants before?
> 
> PX30 and RK3326 are different chips. PX30 has more features. You can simply
> think that RK3326 is a subset of PX30. The RK3326 is more like a PX30
> derivative chip. This is not the same as the previous chips. So we use PX30
> instead of RK3326 for name, and the opening document is only for PX30, we
> think this is more convenient for developers.

ok, sounds reasonable :-) ... So we'll stay with the PX30.


Heiko

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Andrew Lunn @ 2018-05-13 20:29 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <CAGb2v64ACc1tQRhy4LhnvCC7VJ3nVM-wMbgUgzviGMWKPFsSuQ@mail.gmail.com>

On Sun, May 13, 2018 at 01:11:08PM -0700, Chen-Yu Tsai wrote:
> On Sun, May 13, 2018 at 1:05 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> > Hi Chen-Yu
> >> >
> >> > Are these delays the MAC applies? Not the PHY. It would be good to
> >> > make it clear here these are MAC imposed delays.
> >>
> >> Yes these are applied on the MAC side. Being described in the device
> >> tree bindings for the MAC, I thought this was implied to be the case?
> >> Are there known exceptions?
> >
> > There is frequent confusion with this. Most of the time, the PHY does
> > the delay, not the MAC, based on the phy-mode. So the MAC doing it is
> > an exception in itself.
> >
> > Do you actually need these delays for the board you adding support
> > for? Does the PHY not support adding the needed delays? If you don't
> > need the delays, i would not even implement them.
> 
> Yes this is already used on the Bananapi M3. This patch merely reformats
> the description and adds a note saying this only applies to RGMII mode.

Yes, the current code is needed for the Bananapi M3. But you have
another patch which extends the code to support a smaller range. Do
you have a board which actually needs this? If not, i would not add
that new code.

     Andrew

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Andrew Lunn @ 2018-05-13 20:27 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: linux-arm-kernel, Chen-Yu Tsai, devicetree, Maxime Ripard, netdev,
	Rob Herring, Corentin Labbe, Giuseppe Cavallaro
In-Reply-To: <272E1914-029C-492C-B521-BC63D6A705D7@aosc.io>

On Mon, May 14, 2018 at 04:10:58AM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2018年5月14日 GMT+08:00 上午4:05:29, Andrew Lunn <andrew@lunn.ch> 写到:
> >> > Hi Chen-Yu
> >> >
> >> > Are these delays the MAC applies? Not the PHY. It would be good to
> >> > make it clear here these are MAC imposed delays.
> >> 
> >> Yes these are applied on the MAC side. Being described in the device
> >> tree bindings for the MAC, I thought this was implied to be the case?
> >> Are there known exceptions?
> >
> >There is frequent confusion with this. Most of the time, the PHY does
> >the delay, not the MAC, based on the phy-mode. So the MAC doing it is
> >an exception in itself.
> >
> >Do you actually need these delays for the board you adding support
> >for? Does the PHY not support adding the needed delays? If you don't
> >need the delays, i would not even implement them.
> 
> They are needed if the rx/tx have not equal length.
> 

> For several boards if the delay is not present, the network
> is unusable. (I have at least met this on Banana Pi M3, an
> Allwinner A83T SBC w/ RTL8211E)

The RTL8211E does not appear to support adding delays, via
registers. You can do it with strapping resistors. The RTL8211F does
support delays via a register. So for the Banana Pi M3 it is
justified.

However, in general, i would recommend letting the PHY do it, if it
can.

	Andrew

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Chen-Yu Tsai @ 2018-05-13 20:11 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513200529.GF12738@lunn.ch>

On Sun, May 13, 2018 at 1:05 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > Hi Chen-Yu
>> >
>> > Are these delays the MAC applies? Not the PHY. It would be good to
>> > make it clear here these are MAC imposed delays.
>>
>> Yes these are applied on the MAC side. Being described in the device
>> tree bindings for the MAC, I thought this was implied to be the case?
>> Are there known exceptions?
>
> There is frequent confusion with this. Most of the time, the PHY does
> the delay, not the MAC, based on the phy-mode. So the MAC doing it is
> an exception in itself.
>
> Do you actually need these delays for the board you adding support
> for? Does the PHY not support adding the needed delays? If you don't
> need the delays, i would not even implement them.

Yes this is already used on the Bananapi M3. This patch merely reformats
the description and adds a note saying this only applies to RGMII mode.

ChenYu

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Icenowy Zheng @ 2018-05-13 20:10 UTC (permalink / raw)
  To: Andrew Lunn, Chen-Yu Tsai
  Cc: devicetree, Maxime Ripard, netdev, Rob Herring, Corentin Labbe,
	Giuseppe Cavallaro, linux-arm-kernel
In-Reply-To: <20180513200529.GF12738@lunn.ch>



于 2018年5月14日 GMT+08:00 上午4:05:29, Andrew Lunn <andrew@lunn.ch> 写到:
>> > Hi Chen-Yu
>> >
>> > Are these delays the MAC applies? Not the PHY. It would be good to
>> > make it clear here these are MAC imposed delays.
>> 
>> Yes these are applied on the MAC side. Being described in the device
>> tree bindings for the MAC, I thought this was implied to be the case?
>> Are there known exceptions?
>
>There is frequent confusion with this. Most of the time, the PHY does
>the delay, not the MAC, based on the phy-mode. So the MAC doing it is
>an exception in itself.
>
>Do you actually need these delays for the board you adding support
>for? Does the PHY not support adding the needed delays? If you don't
>need the delays, i would not even implement them.

They are needed if the rx/tx have not equal length.

For several boards if the delay is not present, the network
is unusable. (I have at least met this on Banana Pi M3, an
Allwinner A83T SBC w/ RTL8211E)

>
>    Andrew
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Andrew Lunn @ 2018-05-13 20:05 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <CAGb2v66nqWnioS1pwfWFtMZFZSWvf4SwXr_ExnnGvYsK9RU_LA@mail.gmail.com>

> > Hi Chen-Yu
> >
> > Are these delays the MAC applies? Not the PHY. It would be good to
> > make it clear here these are MAC imposed delays.
> 
> Yes these are applied on the MAC side. Being described in the device
> tree bindings for the MAC, I thought this was implied to be the case?
> Are there known exceptions?

There is frequent confusion with this. Most of the time, the PHY does
the delay, not the MAC, based on the phy-mode. So the MAC doing it is
an exception in itself.

Do you actually need these delays for the board you adding support
for? Does the PHY not support adding the needed delays? If you don't
need the delays, i would not even implement them.

    Andrew

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Chen-Yu Tsai @ 2018-05-13 19:53 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513194919.GE12738@lunn.ch>

On Sun, May 13, 2018 at 12:49 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, May 14, 2018 at 03:14:18AM +0800, Chen-Yu Tsai wrote:
>> The clock delay chains found in the glue layer for dwmac-sun8i are only
>> used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
>> MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
>> range of possible values for the delay chain.
>>
>> This patch reformats the delay chain section of the device tree binding
>> to make it clear that the delay chains only apply to RGMII PHYs, and
>> make it easier to add the R40-specific bits later.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
>> ---
>>  Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> index 3d6d5fa0c4d5..e04ce75e24a3 100644
>> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> @@ -28,10 +28,13 @@ Required properties:
>>    - allwinner,sun8i-a83t-system-controller
>>
>>  Optional properties:
>> -- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
>> -- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
>> -Both delay properties need to be a multiple of 100. They control the delay for
>> -external PHY.
>> +- allwinner,tx-delay-ps: TX clock delay chain value in ps.
>> +                      Range is 0-700. Default is 0.
>> +- allwinner,rx-delay-ps: RX clock delay chain value in ps.
>> +                      Range is 0-3100. Default is 0.
>> +Both delay properties need to be a multiple of 100. They control the
>> +clock delay for external RGMII PHY. They do not apply to the internal
>> +PHY or external non-RGMII PHYs.
>
> Hi Chen-Yu
>
> Are these delays the MAC applies? Not the PHY. It would be good to
> make it clear here these are MAC imposed delays.

Yes these are applied on the MAC side. Being described in the device
tree bindings for the MAC, I thought this was implied to be the case?
Are there known exceptions?

Thanks
ChenYu

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Andrew Lunn @ 2018-05-13 19:49 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-2-wens@csie.org>

On Mon, May 14, 2018 at 03:14:18AM +0800, Chen-Yu Tsai wrote:
> The clock delay chains found in the glue layer for dwmac-sun8i are only
> used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
> MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
> range of possible values for the delay chain.
> 
> This patch reformats the delay chain section of the device tree binding
> to make it clear that the delay chains only apply to RGMII PHYs, and
> make it easier to add the R40-specific bits later.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 3d6d5fa0c4d5..e04ce75e24a3 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -28,10 +28,13 @@ Required properties:
>    - allwinner,sun8i-a83t-system-controller
>  
>  Optional properties:
> -- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
> -- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
> -Both delay properties need to be a multiple of 100. They control the delay for
> -external PHY.
> +- allwinner,tx-delay-ps: TX clock delay chain value in ps.
> +			 Range is 0-700. Default is 0.
> +- allwinner,rx-delay-ps: RX clock delay chain value in ps.
> +			 Range is 0-3100. Default is 0.
> +Both delay properties need to be a multiple of 100. They control the
> +clock delay for external RGMII PHY. They do not apply to the internal
> +PHY or external non-RGMII PHYs.

Hi Chen-Yu

Are these delays the MAC applies? Not the PHY. It would be good to
make it clear here these are MAC imposed delays.

     Andrew

^ permalink raw reply

* Re: [PATCH net-next v2 15/15] arm64: dts: allwinner: a64: add SRAM controller device tree node
From: Chen-Yu Tsai @ 2018-05-13 19:37 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Icenowy Zheng, linux-arm-kernel, Mark Rutland, devicetree,
	Stephen Boyd, netdev, Michael Turquette, Rob Herring,
	Corentin Labbe, Mark Brown, Giuseppe Cavallaro, linux-clk
In-Reply-To: <20180502115437.zlu2rafrguufutvp@flea>

On Wed, May 2, 2018 at 4:54 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> On Wed, May 02, 2018 at 06:19:51PM +0800, Icenowy Zheng wrote:
>>
>>
>> 于 2018年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到:
>> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard
>> ><maxime.ripard@bootlin.com> wrote:
>> >> Hi,
>> >>
>> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai wrote:
>> >>> From: Icenowy Zheng <icenowy@aosc.io>
>> >>>
>> >>> Allwinner A64 has a SRAM controller, and in the device tree
>> >currently
>> >>> we have a syscon node to enable EMAC driver to access the EMAC clock
>> >>> register. As SRAM controller driver can now export regmap for this
>> >>> register, replace the syscon node to the SRAM controller device
>> >node,
>> >>> and let EMAC driver to acquire its EMAC clock regmap.
>> >>>
>> >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> >>> ---
>> >>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23
>> >+++++++++++++++----
>> >>>  1 file changed, 19 insertions(+), 4 deletions(-)
>> >>>
>> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >>> index 1b2ef28c42bd..1c37659d9d41 100644
>> >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >>> @@ -168,10 +168,25 @@
>> >>>               #size-cells = <1>;
>> >>>               ranges;
>> >>>
>> >>> -             syscon: syscon@1c00000 {
>> >>> -                     compatible =
>> >"allwinner,sun50i-a64-system-controller",
>> >>> -                             "syscon";
>> >>> +             sram_controller: sram-controller@1c00000 {
>> >>> +                     compatible =
>> >"allwinner,sun50i-a64-sram-controller";
>> >>
>> >> I don't think there's anything preventing us from keeping the
>> >> -system-controller compatible. It's what was in the DT before, and
>> >> it's how it's called in the datasheet.
>> >
>> >I actually meant to ask you about this. The -system-controller
>> >compatible matches the datasheet better. Maybe we should just
>> >switch to that one?
>>
>> No, if we do the switch the system-controller compatible,
>> the device will be probed on the same memory region with
>> a syscon on old DTs.
>
> The device hasn't magically changed either. Maybe we just need to add
> a check to make sure we don't have the syscon compatible in the SRAM
> driver probe so that the double driver issue doesn't happen?

The syscon interface (which is not even a full blown device driver)
only looks at the "syscon" compatible. Either way we're removing that
part from the device tree so things should be ok for new device trees.
As Maxime mentioned we can do a check for the syscon compatible and
either give a warning to the user asking them to update their device
tree, or not register our custom regmap, or not probe the SRAM driver.
Personally I prefer the first option. The system controller block is
probed before any syscon users, so we should be fine, given the dwmac
driver goes the custom regmap path first.

BTW, I still might end up changing the compatible. The manual uses
"system control", not "system controller", which I think makes sense,
since it is just a bunch of register files, kind of like the GRF
(General Register Files) block found in Rockchip SoCs [1], and not an
actual "controller".

ChenYu

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/soc/rockchip/grf.txt

^ permalink raw reply

* [PATCH RESEND net-next v2 8/8] net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds support for it using the framework laid out by previous
patches to map the differences.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c   | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4f5612a3c855..2f7f0915f071 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -93,6 +93,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
 	.msb = 31,
 };
 
+/* EMAC clock register @ 0x164 in the CCU address range */
+static const struct reg_field sun8i_ccu_reg_field = {
+	.reg = 0x164,
+	.lsb = 0,
+	.msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
 	.default_syscon_value = 0x58000,
 	.syscon_field = &sun8i_syscon_reg_field,
@@ -121,6 +128,14 @@ static const struct emac_variant emac_variant_a83t = {
 	.tx_delay_max = 7,
 };
 
+static const struct emac_variant emac_variant_r40 = {
+	.default_syscon_value = 0,
+	.syscon_field = &sun8i_ccu_reg_field,
+	.support_mii = true,
+	.support_rgmii = true,
+	.rx_delay_max = 7,
+};
+
 static const struct emac_variant emac_variant_a64 = {
 	.default_syscon_value = 0,
 	.syscon_field = &sun8i_syscon_reg_field,
@@ -1160,6 +1175,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
 		.data = &emac_variant_v3s },
 	{ .compatible = "allwinner,sun8i-a83t-emac",
 		.data = &emac_variant_a83t },
+	{ .compatible = "allwinner,sun8i-r40-gmac",
+		.data = &emac_variant_r40 },
 	{ .compatible = "allwinner,sun50i-a64-emac",
 		.data = &emac_variant_a64 },
 	{ }
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 7/8] net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay chains
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

On the R40 SoC, the RX delay chain only has a range of 0~7 (hundred
picoseconds), instead of 0~31. Also the TX delay chain is completely
absent.

This patch adds support for different ranges by adding per-compatible
maximum values in the variant data. A maximum of 0 indicates that the
delay chain is not supported or absent.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 32 +++++++++++++------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 79e104a20e20..4f5612a3c855 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -47,6 +47,12 @@
  * @support_mii:		Does the MAC handle MII
  * @support_rmii:		Does the MAC handle RMII
  * @support_rgmii:		Does the MAC handle RGMII
+ *
+ * @rx_delay_max:		Maximum raw value for RX delay chain
+ * @tx_delay_max:		Maximum raw value for TX delay chain
+ *				These two also indicate the bitmask for
+ *				the RX and TX delay chain registers. A
+ *				value of zero indicates this is not supported.
  */
 struct emac_variant {
 	u32 default_syscon_value;
@@ -55,6 +61,8 @@ struct emac_variant {
 	bool support_mii;
 	bool support_rmii;
 	bool support_rgmii;
+	u8 rx_delay_max;
+	u8 tx_delay_max;
 };
 
 /* struct sunxi_priv_data - hold all sunxi private data
@@ -91,7 +99,9 @@ static const struct emac_variant emac_variant_h3 = {
 	.soc_has_internal_phy = true,
 	.support_mii = true,
 	.support_rmii = true,
-	.support_rgmii = true
+	.support_rgmii = true,
+	.rx_delay_max = 31,
+	.tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_v3s = {
@@ -106,7 +116,9 @@ static const struct emac_variant emac_variant_a83t = {
 	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = false,
 	.support_mii = true,
-	.support_rgmii = true
+	.support_rgmii = true,
+	.rx_delay_max = 31,
+	.tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_a64 = {
@@ -115,7 +127,9 @@ static const struct emac_variant emac_variant_a64 = {
 	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rmii = true,
-	.support_rgmii = true
+	.support_rgmii = true,
+	.rx_delay_max = 31,
+	.tx_delay_max = 7,
 };
 
 #define EMAC_BASIC_CTL0 0x00
@@ -219,9 +233,7 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_RMII_EN		BIT(13) /* 1: enable RMII (overrides EPIT) */
 
 /* Generic system control EMAC_CLK bits */
-#define SYSCON_ETXDC_MASK		GENMASK(2, 0)
 #define SYSCON_ETXDC_SHIFT		10
-#define SYSCON_ERXDC_MASK		GENMASK(4, 0)
 #define SYSCON_ERXDC_SHIFT		5
 /* EMAC PHY Interface Type */
 #define SYSCON_EPIT			BIT(2) /* 1: RGMII, 0: MII */
@@ -847,8 +859,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 		}
 		val /= 100;
 		dev_dbg(priv->device, "set tx-delay to %x\n", val);
-		if (val <= SYSCON_ETXDC_MASK) {
-			reg &= ~(SYSCON_ETXDC_MASK << SYSCON_ETXDC_SHIFT);
+		if (val <= gmac->variant->tx_delay_max) {
+			reg &= ~(gmac->variant->tx_delay_max <<
+				 SYSCON_ETXDC_SHIFT);
 			reg |= (val << SYSCON_ETXDC_SHIFT);
 		} else {
 			dev_err(priv->device, "Invalid TX clock delay: %d\n",
@@ -864,8 +877,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 		}
 		val /= 100;
 		dev_dbg(priv->device, "set rx-delay to %x\n", val);
-		if (val <= SYSCON_ERXDC_MASK) {
-			reg &= ~(SYSCON_ERXDC_MASK << SYSCON_ERXDC_SHIFT);
+		if (val <= gmac->variant->rx_delay_max) {
+			reg &= ~(gmac->variant->rx_delay_max <<
+				 SYSCON_ERXDC_SHIFT);
 			reg |= (val << SYSCON_ERXDC_SHIFT);
 		} else {
 			dev_err(priv->device, "Invalid RX clock delay: %d\n",
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 6/8] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external device
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU
address space. Using a standard syscon to access it provides no
coordination with the CCU driver for register access. Neither does
it prevent this and other drivers from accessing other, maybe critical,
clock control registers. On other SoCs, the register is in the "system
control" address space, which might also contain controls for mapping
SRAM to devices or the CPU. This hardware has the same issues.

Instead, for these types of setups, we let the device containing the
control register create a regmap tied to it. We can then get the device
from the existing syscon phandle, and retrieve the regmap with
dev_get_regmap().

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 50 ++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index bbc051474806..79e104a20e20 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -983,6 +983,34 @@ static struct mac_device_info *sun8i_dwmac_setup(void *ppriv)
 	return mac;
 }
 
+static struct regmap *sun8i_dwmac_get_syscon_from_dev(struct device_node *node)
+{
+	struct device_node *syscon_node;
+	struct platform_device *syscon_pdev;
+	struct regmap *regmap = NULL;
+
+	syscon_node = of_parse_phandle(node, "syscon", 0);
+	if (!syscon_node)
+		return ERR_PTR(-ENODEV);
+
+	syscon_pdev = of_find_device_by_node(syscon_node);
+	if (!syscon_pdev) {
+		/* platform device might not be probed yet */
+		regmap = ERR_PTR(-EPROBE_DEFER);
+		goto out_put_node;
+	}
+
+	/* If no regmap is found then the other device driver is at fault */
+	regmap = dev_get_regmap(&syscon_pdev->dev, NULL);
+	if (!regmap)
+		regmap = ERR_PTR(-EINVAL);
+
+	platform_device_put(syscon_pdev);
+out_put_node:
+	of_node_put(syscon_node);
+	return regmap;
+}
+
 static int sun8i_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;
@@ -1027,7 +1055,27 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 		gmac->regulator = NULL;
 	}
 
-	regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon");
+	/* The "GMAC clock control" register might be located in the
+	 * CCU address range (on the R40), or the system control address
+	 * range (on most other sun8i and later SoCs).
+	 *
+	 * The former controls most if not all clocks in the SoC. The
+	 * latter has an SoC identification register, and on some SoCs,
+	 * controls to map device specific SRAM to either the intended
+	 * peripheral, or the CPU address space.
+	 *
+	 * In either case, there should be a coordinated and restricted
+	 * method of accessing the register needed here. This is done by
+	 * having the device export a custom regmap, instead of a generic
+	 * syscon, which grants all access to all registers.
+	 *
+	 * To support old device trees, we fall back to using the syscon
+	 * interface if possible.
+	 */
+	regmap = sun8i_dwmac_get_syscon_from_dev(pdev->dev.of_node);
+	if (IS_ERR(regmap))
+		regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							 "syscon");
 	if (IS_ERR(regmap)) {
 		ret = PTR_ERR(regmap);
 		dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 5/8] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

On the Allwinner R40, the "GMAC clock" register is located in the CCU
block, at a different register address than the other SoCs that have
it in the "system control" block.

This patch converts the use of regmap to regmap_field for mapping and
accessing the syscon register, so we can have the register address in
the variants data, and not in the actual register manipulation code.

This patch only converts regmap_read() and regmap_write() calls to
regmap_field_read() and regmap_field_write() calls. There are some
places where it might make sense to switch to regmap_field_update_bits(),
but this is not done here to keep the patch simple.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 42 ++++++++++++++-----
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a3fa65b1ca8e..bbc051474806 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -42,6 +42,7 @@
  *				This value is used for disabling properly EMAC
  *				and used as a good starting value in case of the
  *				boot process(uboot) leave some stuff.
+ * @syscon_field		reg_field for the syscon's gmac register
  * @soc_has_internal_phy:	Does the MAC embed an internal PHY
  * @support_mii:		Does the MAC handle MII
  * @support_rmii:		Does the MAC handle RMII
@@ -49,6 +50,7 @@
  */
 struct emac_variant {
 	u32 default_syscon_value;
+	const struct reg_field *syscon_field;
 	bool soc_has_internal_phy;
 	bool support_mii;
 	bool support_rmii;
@@ -71,13 +73,21 @@ struct sunxi_priv_data {
 	struct regulator *regulator;
 	struct reset_control *rst_ephy;
 	const struct emac_variant *variant;
-	struct regmap *regmap;
+	struct regmap_field *regmap_field;
 	bool internal_phy_powered;
 	void *mux_handle;
 };
 
+/* EMAC clock register @ 0x30 in the "system control" address range */
+static const struct reg_field sun8i_syscon_reg_field = {
+	.reg = 0x30,
+	.lsb = 0,
+	.msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
 	.default_syscon_value = 0x58000,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = true,
 	.support_mii = true,
 	.support_rmii = true,
@@ -86,12 +96,14 @@ static const struct emac_variant emac_variant_h3 = {
 
 static const struct emac_variant emac_variant_v3s = {
 	.default_syscon_value = 0x38000,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = true,
 	.support_mii = true
 };
 
 static const struct emac_variant emac_variant_a83t = {
 	.default_syscon_value = 0,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rgmii = true
@@ -99,6 +111,7 @@ static const struct emac_variant emac_variant_a83t = {
 
 static const struct emac_variant emac_variant_a64 = {
 	.default_syscon_value = 0,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rmii = true,
@@ -216,7 +229,6 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_ETCS_MII		0x0
 #define SYSCON_ETCS_EXT_GMII	0x1
 #define SYSCON_ETCS_INT_GMII	0x2
-#define SYSCON_EMAC_REG		0x30
 
 /* sun8i_dwmac_dma_reset() - reset the EMAC
  * Called from stmmac via stmmac_dma_ops->reset
@@ -745,7 +757,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
 	bool need_power_ephy = false;
 
 	if (current_child ^ desired_child) {
-		regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
+		regmap_field_read(gmac->regmap_field, &reg);
 		switch (desired_child) {
 		case DWMAC_SUN8I_MDIO_MUX_INTERNAL_ID:
 			dev_info(priv->device, "Switch mux to internal PHY");
@@ -763,7 +775,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
 				desired_child);
 			return -EINVAL;
 		}
-		regmap_write(gmac->regmap, SYSCON_EMAC_REG, val);
+		regmap_field_write(gmac->regmap_field, val);
 		if (need_power_ephy) {
 			ret = sun8i_dwmac_power_internal_phy(priv);
 			if (ret)
@@ -801,7 +813,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 	int ret;
 	u32 reg, val;
 
-	regmap_read(gmac->regmap, SYSCON_EMAC_REG, &val);
+	regmap_field_read(gmac->regmap_field, &val);
 	reg = gmac->variant->default_syscon_value;
 	if (reg != val)
 		dev_warn(priv->device,
@@ -883,7 +895,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 		return -EINVAL;
 	}
 
-	regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg);
+	regmap_field_write(gmac->regmap_field, reg);
 
 	return 0;
 }
@@ -892,7 +904,7 @@ static void sun8i_dwmac_unset_syscon(struct sunxi_priv_data *gmac)
 {
 	u32 reg = gmac->variant->default_syscon_value;
 
-	regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg);
+	regmap_field_write(gmac->regmap_field, reg);
 }
 
 static void sun8i_dwmac_exit(struct platform_device *pdev, void *priv)
@@ -980,6 +992,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	int ret;
 	struct stmmac_priv *priv;
 	struct net_device *ndev;
+	struct regmap *regmap;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
 	if (ret)
@@ -1014,14 +1027,21 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 		gmac->regulator = NULL;
 	}
 
-	gmac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
-						       "syscon");
-	if (IS_ERR(gmac->regmap)) {
-		ret = PTR_ERR(gmac->regmap);
+	regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon");
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
 		dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
 		return ret;
 	}
 
+	gmac->regmap_field = devm_regmap_field_alloc(dev, regmap,
+						     *gmac->variant->syscon_field);
+	if (IS_ERR(gmac->regmap_field)) {
+		ret = PTR_ERR(gmac->regmap_field);
+		dev_err(dev, "Unable to map syscon register: %d\n", ret);
+		return ret;
+	}
+
 	plat_dat->interface = of_get_phy_mode(dev->of_node);
 
 	/* platform data specifying hardware features and callbacks.
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 4/8] dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40 SoC
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds the R40 specific bits to the dwmac-sun8i binding.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1c0906a5c02b..cfe724398a12 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -7,6 +7,7 @@ Required properties:
 - compatible: must be one of the following string:
 		"allwinner,sun8i-a83t-emac"
 		"allwinner,sun8i-h3-emac"
+		"allwinner,sun8i-r40-gmac"
 		"allwinner,sun8i-v3s-emac"
 		"allwinner,sun50i-a64-emac"
 - reg: address and length of the register for the device.
@@ -25,8 +26,10 @@ Required properties:
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
 			 Range is 0-700. Default is 0.
+			 Unavailable for allwinner,sun8i-r40-gmac
 - allwinner,rx-delay-ps: RX clock delay chain value in ps.
 			 Range is 0-3100. Default is 0.
+			 Range is 0-700 for allwinner,sun8i-r40-gmac
 Both delay properties need to be a multiple of 100. They control the
 clock delay for external RGMII PHY. They do not apply to the internal
 PHY or external non-RGMII PHYs.
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 3/8] dt-bindings: net: dwmac-sun8i: simplify description of syscon property
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The syscon property is used to point to the device that holds the glue
layer control register known as the "EMAC (or GMAC) clock register".

We do not need to explicitly list what compatible strings are needed, as
this information is readily available in the user manuals. Also the
"syscon" device type is more of an implementation detail. There are many
ways to access a register not in a device's address range, the syscon
interface being the most generic and unrestricted one.

Simplify the description so that it says what it is supposed to
describe.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1b8e33e71651..1c0906a5c02b 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -20,12 +20,7 @@ Required properties:
 - phy-handle: See ethernet.txt
 - #address-cells: shall be 1
 - #size-cells: shall be 0
-- syscon: A phandle to the syscon of the SoC with one of the following
- compatible string:
-  - allwinner,sun8i-a83t-system-controller
-  - allwinner,sun8i-h3-system-controller
-  - allwinner,sun8i-v3s-system-controller
-  - allwinner,sun50i-a64-system-controller
+- syscon: A phandle to the device containing the EMAC or GMAC clock register
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 2/8] dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: devicetree, Maxime Ripard, netdev, Chen-Yu Tsai, Rob Herring,
	Corentin Labbe, linux-arm-kernel, Icenowy Zheng
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The A83T syscon compatible was appended to the syscon compatibles list,
instead of inserted in to preserve the ordering.

Move it to the proper place to keep the list sorted.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index e04ce75e24a3..1b8e33e71651 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -22,10 +22,10 @@ Required properties:
 - #size-cells: shall be 0
 - syscon: A phandle to the syscon of the SoC with one of the following
  compatible string:
+  - allwinner,sun8i-a83t-system-controller
   - allwinner,sun8i-h3-system-controller
   - allwinner,sun8i-v3s-system-controller
   - allwinner,sun50i-a64-system-controller
-  - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The clock delay chains found in the glue layer for dwmac-sun8i are only
used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
range of possible values for the delay chain.

This patch reformats the delay chain section of the device tree binding
to make it clear that the delay chains only apply to RGMII PHYs, and
make it easier to add the R40-specific bits later.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 3d6d5fa0c4d5..e04ce75e24a3 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -28,10 +28,13 @@ Required properties:
   - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
-- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
-- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
-Both delay properties need to be a multiple of 100. They control the delay for
-external PHY.
+- allwinner,tx-delay-ps: TX clock delay chain value in ps.
+			 Range is 0-700. Default is 0.
+- allwinner,rx-delay-ps: RX clock delay chain value in ps.
+			 Range is 0-3100. Default is 0.
+Both delay properties need to be a multiple of 100. They control the
+clock delay for external RGMII PHY. They do not apply to the internal
+PHY or external non-RGMII PHYs.
 
 Optional properties for the following compatibles:
   - "allwinner,sun8i-h3-emac",
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 0/8] net: stmmac: dwmac-sun8i: Support R40
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring

This is a resend of the patches for net-next split out from my R40
Ethernet support v2 series, as requested by David Miller. The arm-soc
bits will follow, once I rework the A64 system controller compatible.

Patches 1, 2, and 3 clean up the dwmac-sun8i binding.

Patch 4 adds device tree binding for Allwinner R40's Ethernet
controller.

Patch 5 converts regmap access of the syscon region in the dwmac-sun8i
driver to regmap_field, in anticipation of different field widths on
the R40.

Patch 6 introduces custom plumbing in the dwmac-sun8i driver to fetch
a regmap from another device, by looking up said device via a phandle,
then getting the regmap associated with that device.

Patch 7 adds support for different or absent TX/RX delay chain ranges
to the dwmac-sun8i driver.

Patch 8 adds support for the R40's ethernet controller.


Excerpt from original cover letter:

Changes since v1:

  - Default to fetching regmap from device pointed to by syscon phandle,
    and falling back to syscon API if that fails.

  - Dropped .syscon_from_dev field in device data as a result of the
    previous change.

  - Added a large comment block explaining the first change.

  - Simplified description of syscon property in sun8i-dwmac binding.

  - Regmap now only exposes the EMAC/GMAC register, but retains the
    offset within its address space.

  - Added patches for A64, which reuse the same sun8i-dwmac changes.

This series adds support for the DWMAC based Ethernet controller found
on the Allwinner R40 SoC. The controller is either a DWMAC clone or
DWMAC core with its registers rearranged. This is already supported by
the dwmac-sun8i driver. The glue layer control registers, unlike other
sun8i family SoCs, is not in the system controller region, but in the
clock control unit, like with the older A20 and A31 SoCs.

While we reuse the bindings for dwmac-sun8i using a syscon phandle
reference, we need some custom plumbing for the clock driver to export
a regmap that only allows access to the GMAC register to the dwmac-sun8i
driver. An alternative would be to allow drivers to register custom
syscon devices with their own regmap and locking.


Please have a look.

Regards
ChenYu

Chen-Yu Tsai (8):
  dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
  dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical
    order
  dt-bindings: net: dwmac-sun8i: simplify description of syscon property
  dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40
    SoC
  net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access
  net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external
    device
  net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay
    chains
  net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC

 .../devicetree/bindings/net/dwmac-sun8i.txt   |  21 +--
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 139 +++++++++++++++---
 2 files changed, 130 insertions(+), 30 deletions(-)

-- 
2.17.0

^ permalink raw reply

* [PATCH] media: dt-bindings: media: rcar_vin: add support for r8a77965
From: Niklas Söderlund @ 2018-05-13 18:58 UTC (permalink / raw)
  To: Rob Herring, devicetree
  Cc: linux-media, linux-renesas-soc, Niklas Söderlund

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1c669eb35..c2c57dcf73f4851b 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -21,6 +21,7 @@ on Gen3 platforms to a CSI-2 receiver.
    - "renesas,vin-r8a7794" for the R8A7794 device
    - "renesas,vin-r8a7795" for the R8A7795 device
    - "renesas,vin-r8a7796" for the R8A7796 device
+   - "renesas,vin-r8a77965" for the R8A77965 device
    - "renesas,vin-r8a77970" for the R8A77970 device
    - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
      device.
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4
From: jacopo mondi @ 2018-05-13 18:30 UTC (permalink / raw)
  To: Simon Horman
  Cc: Niklas Söderlund, Jacopo Mondi, laurent.pinchart, geert,
	magnus.damm, robh+dt, linux-renesas-soc, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20180511134516.omsv25i2wi4cxypc@verge.net.au>

Hi Simon,

On Fri, May 11, 2018 at 03:45:16PM +0200, Simon Horman wrote:
> On Fri, May 11, 2018 at 01:25:23PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> > 
> > Thanks for your work.
> > 
> > On 2018-05-11 12:00:01 +0200, Jacopo Mondi wrote:
> > > Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> > > 
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > 
> > Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > 
> > > ---
> > >  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > index 82aed7e..bdf7017 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > @@ -783,6 +783,17 @@
> > >  				};
> > >  			};
> > >  		};
> > > +
> > > +		vin4: video@e6ef4000 {
> > > +			compatible = "renesas,vin-r8a77995";
> > > +			reg = <0 0xe6ef4000 0 0x1000>;
> > > +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&cpg CPG_MOD 807>;
> > > +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> > > +			resets = <&cpg 807>;
> > > +			renesas,id = <4>;
> > > +			status = "disabled";
> > > +		};
> > >  	};
> 
> Thanks, I have moved the new node to preserve sorting of nodes by bus
> address and applied the result. It is as follows:

Great, thanks for doing this, I should have take care of sorting nodes
opprtunely.

Thanks
   j

> 
> From: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Subject: [PATCH] arm64: dts: renesas: r8a77995: Add VIN4
> 
> Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> [simon: sorted node by bus address]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> index ba98865b0c9b..2506f46293e8 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -610,6 +610,17 @@
>  			status = "disabled";
>  		};
s  
> +		vin4: video@e6ef4000 {
> +			compatible = "renesas,vin-r8a77995";
> +			reg = <0 0xe6ef4000 0 0x1000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 807>;
> +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> +			resets = <&cpg 807>;
> +			renesas,id = <4>;
> +			status = "disabled";
> +		};
> +
>  		ohci0: usb@ee080000 {
>  			compatible = "generic-ohci";
>  			reg = <0 0xee080000 0 0x100>;
> -- 
> 2.11.0
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox