Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support
From: Jan Lübbe @ 2016-11-30 11:01 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev, Mugunthan V N, Richard Cochran,
	Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok
In-Reply-To: <20161128230428.6872-5-grygorii.strashko@ti.com>

On Mo, 2016-11-28 at 17:04 -0600, Grygorii Strashko wrote:
> --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
> +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
> @@ -127,6 +127,16 @@ Optional properties:
>                 The number of external time stamp channels.
>                 The different CPTS versions might support up 8
>                 external time stamp channels. if absent - unsupported.
> +       - cpts-ts-comp-length:
> +               Enable time stamp comparison event and TS_COMP signal output
> +               generation when CPTS counter reaches a value written to
> +               the TS_COMP_VAL register.
> +               The generated pulse width is 3 refclk cycles if this property
> +               has no value (empty) or, otherwise, it should specify desired
> +               pulse width in number of refclk periods - max value 2^16.
> +               TS_COMP functionality will be disabled if not present.
> +       - cpts-ts-comp-polarity-low:
> +               Set polarity of TS_COMP signal to low. Default is hight.

Why is this configured via DT? Are the values fixed for a given board,
depending on external components? Couldn't this be configured somewhere
else?

Regards,
Jan
-- 
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

* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Jean-Francois Moine @ 2016-11-30 10:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
	Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <3478036.d2UQM8n6lv@avalon>

On Wed, 30 Nov 2016 11:52:25 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:

> Hi Jean-François,
> 
> On Wednesday 30 Nov 2016 10:27:57 Jean-Francois Moine wrote:
> > On Wed, 30 Nov 2016 10:20:21 +0200 Laurent Pinchart wrote:
> > >> Well, I don't see what this connector can be.
> > >> May you give me a DT example?
> > > 
> > > Sure.
> > > 
> > > arch/arm/boot/dts/r8a7791-koelsch.dts
> > > 
> > >         /* HDMI encoder */
	[snip]
> > >         /* HDMI connector */
> > >         
> > >         hdmi-out {
> > >                 compatible = "hdmi-connector";
> > >                 type = "a";
> > >                 
> > >                 port {
> > >                         hdmi_con: endpoint {
> > >                                 remote-endpoint = <&adv7511_out>;
> > >                         };
> > >                 };
> > >         };
	[snip]
> > - what does the software do with the connector type?
> 
> That's up to the software to decide, the DT bindings should describe the 
> hardware in the most accurate and usable way for the OS as possible. One of my 
> longer term goals is to add connector drivers to handle DDC and HPD when 
> they're not handled by the encoder (they are in the above example).
> 
> If the DDC was connected to a general-purpose I2C bus of the SoC, and the HPD 
> to a GPIO, we would have
> 
> 	hdmi-out {
> 		compatible = "hdmi-connector";
> 		type = "a";
> 		/* I2C bus and GPIO references are made up for the example */
> 		ddc-i2c-bus = <&i2c4>;
> 		hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
> 
> 		port {
> 			hdmi_con: endpoint {
> 				remote-endpoint = <&adv7511_out>;
> 			};
> 		};
> 	};
> 
> and both HPD and EDID reading should be handled by the connector driver.
	[snip]

Hi Laurent,

OK. I understand. This connector complexity should be added in all DTs,
and the same code would be used.

Actually, for component binding, I use drm_of_component_probe():

- from the DRM master, loop on the "ports" phandle array and bind the
  CRTCs,

- for each CRTC, loop on the first remote port level and bind the
  encoders/connectors

Now, this should be:

- from the DRM master, loop on the first remote ports level and bind
  the CRTCs,

- for each CRTC, loop on the second remote port level and bind the
  encoders (and bridges?),

- for each encoder, loop on the third remote port level and bind the
  connectors.

Then, it would be nice to have a generic function for doing this job.

Otherwise, from your description:

> 	hdmi-out {
> 		compatible = "hdmi-connector";
> 		type = "a";
> 		/* I2C bus and GPIO references are made up for the example */
> 		ddc-i2c-bus = <&i2c4>;
> 		hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>

the "hdmi-connector" is a big piece of software. It must handle a lot
of more and more exotic connectors.
So, I hope that you have written a "simple-hdmi-connector" which does
nothing but setting the connector type.
Where is it?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH] arm64: dts: juno: Correct PCI IO window
From: Lorenzo Pieralisi @ 2016-11-30 10:43 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	liviu.dudau-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8
In-Reply-To: <1480452310-29286-1-git-send-email-jeremy.linton-5wv7dgnIgG8@public.gmane.org>

On Tue, Nov 29, 2016 at 02:45:10PM -0600, Jeremy Linton wrote:
> The PCIe root complex on Juno translates the MMIO mapped
> at 0x5f800000 to the PIO address range starting at 0
> (which is common because PIO addresses are generally < 64k).
> Correct the DT to reflect this.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm64/boot/dts/arm/juno-base.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>

> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index 334271a..7d3a2ac 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -393,7 +393,7 @@
>  		#address-cells = <3>;
>  		#size-cells = <2>;
>  		dma-coherent;
> -		ranges = <0x01000000 0x00 0x5f800000 0x00 0x5f800000 0x0 0x00800000>,
> +		ranges = <0x01000000 0x00 0x00000000 0x00 0x5f800000 0x0 0x00800000>,
>  			 <0x02000000 0x00 0x50000000 0x00 0x50000000 0x0 0x08000000>,
>  			 <0x42000000 0x40 0x00000000 0x40 0x00000000 0x1 0x00000000>;
>  		#interrupt-cells = <1>;
> -- 
> 2.5.5
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] Synopsys USB 2.0 Device Controller (UDC) Driver
From: Felipe Balbi @ 2016-11-30 10:40 UTC (permalink / raw)
  To: Raviteja Garimella, Rob Herring, Mark Rutland, Greg Kroah-Hartman
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, John Youn
In-Reply-To: <1480485910-7797-3-git-send-email-raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

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


Hi,

Raviteja Garimella <raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> writes:
> This is driver for Synopsys Designware Cores USB Device
> Controller (UDC) Subsystem with the AMBA Advanced High-Performance
> Bus (AHB). This driver works with Synopsys UDC20 products.
>
> Signed-off-by: Raviteja Garimella <raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

use drivers/usb/dwc2 instead of duplicating it.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply

* Re: [PATCH v5 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700
From: Romain Perier @ 2016-11-30 10:38 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper,
	Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	Thomas Petazzoni, Nadav Haklai, Omri Itach, Shadi Ammouri,
	Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits
In-Reply-To: <20161129211716.GA15594@katana>

Hello,

Le 29/11/2016 à 22:17, Wolfram Sang a écrit :
>> +	if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
>> +		i2c->fm_mask = ICR_BUSMODE_FM;
>> +		i2c->hs_mask = ICR_BUSMODE_HS;
>> +	} else {
>> +		i2c->fm_mask = ICR_FM;
>> +		i2c->hs_mask = ICR_HS;
>> +	}
>>
>>  	*i2c_types = (enum pxa_i2c_types)(of_id->data);
>>
>> @@ -1181,6 +1194,13 @@ static int i2c_pxa_probe_pdata(struct platform_device *pdev,
>>  			i2c->master_code = 0xe;
>>  		i2c->rate = plat->rate;
>>  	}
>> +	if (!strcmp(id->name, "armada-3700-i2c")) {
>> +		i2c->fm_mask = ICR_BUSMODE_FM;
>> +		i2c->hs_mask = ICR_BUSMODE_HS;
>> +	} else {
>> +		i2c->fm_mask = ICR_FM;
>> +		i2c->hs_mask = ICR_HS;
>> +	}
>
> Okay, having the same code twice is not nice as well.
>
> Sorry for missing this in the first review and going a step back, but I
> think now the best solution is to have again a REGS_A3700 struct, but we
> should extend it with new entries for the shifted bits. Then in the init
> code, you can do something like:
>
> 	i2c->fm_mask = pxa_reg_layout[i2c_type].fm_mask ?: ICR_FM;
>
> Makes sense?

Mhhhh... makes sense yes, it is simpler and would remove the duplicated 
code, yes (no no need to modify probe_dt and probe_pdata in this case).
What do you prefer everything in one commit or two seperated commit ? 
(one including the new fields for fm_mask and another one to add support 
for a3700-i2c).

Thanks,
Romain

-- 
Romain Perier, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 3/6] net: ethernet: ti: cpts: add support of cpts HW_TS_PUSH
From: Richard Cochran @ 2016-11-30 10:19 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
	Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok
In-Reply-To: <20161128230428.6872-4-grygorii.strashko-l0cyMroinI0@public.gmane.org>

On Mon, Nov 28, 2016 at 05:04:25PM -0600, Grygorii Strashko wrote:
> +/* HW TS */
> +static int cpts_extts_enable(struct cpts *cpts, u32 index, int on)
> +{
> +	unsigned long flags;
> +	u32 v;
> +
> +	if (index >= cpts->info.n_ext_ts)
> +		return -ENXIO;
> +
> +	if (((cpts->hw_ts_enable & BIT(index)) >> index) == on)
> +		return 0;
> +
> +	spin_lock_irqsave(&cpts->lock, flags);
> +
> +	v = cpts_read32(cpts, control);
> +	if (on) {
> +		v |= BIT(8 + index);
> +		cpts->hw_ts_enable |= BIT(index);
> +	} else {
> +		v &= ~BIT(8 + index);
> +		cpts->hw_ts_enable &= ~BIT(index);
> +	}
> +	cpts_write32(cpts, v, control);
> +
> +	spin_unlock_irqrestore(&cpts->lock, flags);
> +
> +	if (cpts->hw_ts_enable)
> +		/* poll for events faster - evry 200 ms */

every

> +		cpts->ov_check_period =
> +			msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);

Bad indentation.  Use braces {} to contain the comment and assignment
statement.

> +	else
> +		cpts->ov_check_period = cpts->ov_check_period_slow;
> +
> +	mod_delayed_work(system_wq, &cpts->overflow_work,
> +			 cpts->ov_check_period);
> +
> +	return 0;
> +}

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Laurent Pinchart @ 2016-11-30 10:12 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Jean-Francois Moine, Maxime Ripard,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161130100545.44546ed28896cdb9768f68cd-GANU6spQydw@public.gmane.org>

Hi Jean-François,

On Wednesday 30 Nov 2016 10:05:45 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 22:36:50 +0100 Maxime Ripard wrote:
> > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> > > This patchset series adds HDMI video support to the Allwinner
> > > sun8i SoCs which include the display engine 2 (DE2).
> > > The driver contains the code for the A83T and H3 SoCs, and
> > > some H3 boards, but it could be used/extended for other SoCs
> > > (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> > 
> > Honestly, I'm getting a bit worried by the fact that you ignore
> > reviews.
> > 
> > On the important reviews that you got that are to be seen as major
> > 
> > issues that block the inclusion, we have:
> >   - The fact that the HDMI driver is actually just a designware IP,
> >     and while you should use the driver that already exists, you just
> >     duplicated all that code.
> 
> The DW registers in the A83T and H3 are obfuscated, so, the code in
> bridge/DW cannot be used as it is. There should be either a translation
> table or a function to compute the register addresses.

Jernej mentioned there could be a way to restore the original Synopsys memory 
map. If that works then using the dw-hdmi could be an option.

> More, it is not sure that the bridge/DW code would work with Allwinner's
> SoCs.

If it doesn't work and can't be made to work in a non-invasive way they it 
should certainly not be used :-)

> It seems that they got some schematics from DesignWare, but, is it really
> the same hardware?

That's not really relevant, as long as the hardware is compatible, it doesn't 
matter if it's a Synopsys IP or a custom implementation of the HDMI TX with a 
compatible interface.

> Also, if some changes had to be done in the bridge code, I could not check
> if this would break or not the other SoCs.

Nothing new here, all drivers that support multiple SoCs have the same 
problem. That's why we have maintainers, testers and board farms to try and 
catch issues as early as possible.

> Eventually, I went the same way as omap/hdmi5: different driver.

I might try to fix that for OMAP5 at some point, we'll see.

> >   - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> >     graph to model the connection between the display engine and the
> >     TCON. Something that Laurent also pointed out in this version.
> 
> I simply use the drm function drm_of_component_probe().
> If this one is in the DRM core, why should I not use it?
> If it must not be used, it would be nice to mark it as deprecated and
> to update the code of the drivers which are using it.

I haven't used that function so I can't comment here, except for the fact that 
DT bindings are not designed based on a given OS implementation. It should be 
the other way around, the bindings should be designed to clearly describe the 
hardware in a clean and consistent way that can be used by any host software, 
and the Linux helper functions should then be developed based on those 
bindings.

> >   - The fact that you ignored that you needed an HDMI connector node
> >     as a child of the HDMI controller. This has been reported by Rob
> >     (v6) and yet again in this version by Laurent.
> 
> As I don't know what is a DT 'connector', I cannot go further.
> I hope Laurent will give me clearer explanations and a real example.

Done, we can discuss this in that part of the mail thread.

> >   - And finally the fact that we can't have several display engine in
> >     parallel, if needs be. This has happened in the past already on
> >     Allwinner SoCs, so it's definitely something we should consider in
> >     the DT bindings, since we can't break them.
> 
> IIRC, I proposed my driver before yours, and the DE2 is completely
> different from the other display engines.
> What you are telling is "add more code to already complex code and have
> a big driver for all SoCs in each kernels".
> I think it should be better to have small modules, each one treating
> specific hardware, and to let only the needed code in the kernel memory
> at startup time.
> 
> > Until those are fixed, I cannot see how this driver can be merged,
> > unfortunately.
> 
> No problem. I just wanted to help people by giving the job I did on the
> boards I have. My boards are working for almost one year, fine enough
> for I use them as daily desktop computers. I don't want to spend one
> more year for having my code in the Linux kernel: there are so much
> other exciting things to do...

And you're certainly welcome to contribute drivers to the kernel, that's 
always appreciated. Of course, to ensure a reasonable level of quality and 
consistency between drivers, the review process often requires changes to be 
made to the code being submitted. When it comes to drivers I mostly pay 
attention to DT bindings, userspace APIs and modification to common code. 
Driver code itself, as long as it's reasonably clean and doesn't impede 
development of other drivers or impact system security in an adverse way, is 
still important but maybe slightly less so. I'll defer to Maxime to come to an 
agreement on the multiple display engines in parallel problem as I'm not 
familiar with it for the Allwinner platforms.

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH] iio: misc: add a generic regulator driver
From: Lars-Peter Clausen @ 2016-11-30 10:10 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-devicetree, LKML, Kevin Hilman, Patrick Titiano,
	Neil Armstrong
In-Reply-To: <CAMpxmJWDWk-mXUHHGEKm-VVwzqEGLDQjc274Xq+Qwc2eN=YbsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 11/29/2016 04:35 PM, Bartosz Golaszewski wrote:
> 2016-11-29 16:30 GMT+01:00 Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>:
>> On 11/29/2016 04:22 PM, Bartosz Golaszewski wrote:
>> [...]
>>> diff --git a/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt b/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt
>>> new file mode 100644
>>> index 0000000..147458f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt
>>> @@ -0,0 +1,18 @@
>>> +Industrial IO regulator device driver
>>> +-------------------------------------
>>> +
>>> +This document describes the bindings for the iio-regulator - a dummy device
>>> +driver representing a physical regulator within the iio framework.
>>
>> No bindings for drivers, only for hardware. So this wont work.
>>
> 
> What about exporting regulator attributes analogous to the one in this
> patch from the iio-core when a *-supply property is specified for a
> node?

The problem with exposing direct control to the regulator is that it allows
to modify the hardware state without the drivers knowledge. If you
power-cycle a device all previous configuration that has been written to the
device is reset. The device driver needs to be aware of this otherwise its
assumed state and the actual device state can divert which will result in
undefined behavior. Also access to the device will fail unexpectedly when
the regulator is turned off. So I think generally the driver should
explicitly control the regulator, power-up when needed, power-down when not.

- Lars

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: arm64: renesas: r8a7796/salvator-x: Add board part number to DT bindings
From: Simon Horman @ 2016-11-30 10:05 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Magnus Damm, linux-renesas-soc, linux-arm-kernel, devicetree
In-Reply-To: <1472635054-24372-1-git-send-email-geert+renesas@glider.be>

On Wed, Aug 31, 2016 at 11:17:34AM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, queued up for v4.11.

^ permalink raw reply

* Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support
From: Richard Cochran @ 2016-11-30 10:05 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
	linux-omap, Rob Herring, devicetree, Murali Karicheri,
	Wingman Kwok
In-Reply-To: <20161128230428.6872-5-grygorii.strashko@ti.com>

On Mon, Nov 28, 2016 at 05:04:26PM -0600, Grygorii Strashko wrote:
> The TS_COMP output in the CPSW CPTS module is asserted for
> ts_comp_length[15:0] RCLK periods when the time_stamp value compares
> with the ts_comp_val[31:0] and the length value is non-zero. The
> TS_COMP pulse edge occurs three RCLK periods after the values
> compare. A timestamp compare event is pushed into the event FIFO when
> TS_COMP is asserted.
> 
> This patch adds support of Pulse-Per-Second (PPS) by using the
> timestamp compare output. The CPTS driver adds one second of counter
> value to the ts_comp_val register after each assertion of the TS_COMP
> output. The TS_COMP pulse polarity and width are configurable in DT.

I really dislike this patch.  You go through contortions to get from
the timecounter back to the raw HW counter.  That is rather ugly.

Can you adjust the frequency of the keystone devices in hardware?  If
so, then please implement it, and just disable PPS for the CPSW.

The only reason I used the timecounter for frequency adjustment was
because the am335x HW is broken.  But this shouldn't hold back other
newer HW without the same silicon flaws.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH] arm64: dts: juno: Correct PCI IO window
From: liviu.dudau @ 2016-11-30 10:04 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: mark.rutland, devicetree, lorenzo.pieralisi, catalin.marinas,
	will.deacon, robh+dt, sudeep.holla, linux-arm-kernel
In-Reply-To: <1480452310-29286-1-git-send-email-jeremy.linton@arm.com>

On Tue, Nov 29, 2016 at 02:45:10PM -0600, Jeremy Linton wrote:
> The PCIe root complex on Juno translates the MMIO mapped
> at 0x5f800000 to the PIO address range starting at 0
> (which is common because PIO addresses are generally < 64k).
> Correct the DT to reflect this.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

With the U-Boot patch that I have sent to the ML:
Tested-by: Liviu Dudau <Liviu.Dudau@arm.com>

also

Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>

Best regards,
Liviu


> ---
>  arch/arm64/boot/dts/arm/juno-base.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index 334271a..7d3a2ac 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -393,7 +393,7 @@
>  		#address-cells = <3>;
>  		#size-cells = <2>;
>  		dma-coherent;
> -		ranges = <0x01000000 0x00 0x5f800000 0x00 0x5f800000 0x0 0x00800000>,
> +		ranges = <0x01000000 0x00 0x00000000 0x00 0x5f800000 0x0 0x00800000>,
>  			 <0x02000000 0x00 0x50000000 0x00 0x50000000 0x0 0x08000000>,
>  			 <0x42000000 0x40 0x00000000 0x40 0x00000000 0x1 0x00000000>;
>  		#interrupt-cells = <1>;
> -- 
> 2.5.5
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

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

^ permalink raw reply

* Re: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Philipp Zabel @ 2016-11-30 10:03 UTC (permalink / raw)
  To: zhangfei; +Cc: devicetree, Stephen Boyd, Arnd Bergmann, linux-arm-kernel
In-Reply-To: <bec21998-b30c-4076-62bf-6b24c9dd1dd6@linaro.org>

Am Freitag, den 25.11.2016, 20:08 +0800 schrieb zhangfei:
> 
> On 2016年11月25日 18:54, Philipp Zabel wrote:
> > Am Freitag, den 25.11.2016, 18:42 +0800 schrieb zhangfei:
> >> On 2016年11月25日 18:25, Philipp Zabel wrote:
> >>> Am Donnerstag, den 24.11.2016, 18:20 +0800 schrieb zhangfei:
> >>>> On 2016年11月24日 17:50, Philipp Zabel wrote:
> >>>>> Am Donnerstag, den 24.11.2016, 17:40 +0800 schrieb zhangfei:
> >>>>>> On 2016年11月24日 17:26, Philipp Zabel wrote:
> >>>>>>> Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
> >>>>>>>> Add DT bindings documentation for hi3660 SoC reset controller.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> >>>>>>>> ---
> >>>>>>>>      .../bindings/reset/hisilicon,hi3660-reset.txt      | 51 ++++++++++++++++++++++
> >>>>>>>>      1 file changed, 51 insertions(+)
> >>>>>>>>      create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>>
> >>>>>>>> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>> new file mode 100644
> >>>>>>>> index 0000000..250daf2
> >>>>>>>> --- /dev/null
> >>>>>>>> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>> @@ -0,0 +1,51 @@
> >>>>>>>> +Hisilicon System Reset Controller
> >>>>>>>> +======================================
> >>>>>>>> +
> >>>>>>>> +Please also refer to reset.txt in this directory for common reset
> >>>>>>>> +controller binding usage.
> >>>>>>>> +
> >>>>>>>> +The reset controller registers are part of the system-ctl block on
> >>>>>>>> +hi3660 SoC.
> >>>>>>>> +
> >>>>>>>> +Required properties:
> >>>>>>>> +- compatible: should be
> >>>>>>>> +		 "hisilicon,hi3660-reset"
> >>>>>>>> +- #reset-cells: 1, see below
> >>>>>>>> +- hisi,rst-syscon: phandle of the reset's syscon.
> >>>>>>>> +- hisi,reset-bits: Contains the reset control register information
> >>>>>>>> +		  Should contain 2 cells for each reset exposed to
> >>>>>>>> +		  consumers, defined as:
> >>>>>>>> +			Cell #1 : offset from the syscon register base
> >>>>>>>> +			Cell #2 : bits position of the control register
> >>>>>>>> +
> >>>>>>>> +Example:
> >>>>>>>> +	iomcu: iomcu@ffd7e000 {
> >>>>>>>> +		compatible = "hisilicon,hi3660-iomcu", "syscon";
> >>>>>>>> +		reg = <0x0 0xffd7e000 0x0 0x1000>;
> >>>>>>>> +	};
> >>>>>>>> +
> >>>>>>>> +	iomcu_rst: iomcu_rst_controller {
> >>>>>>> This should be
> >>>>>>> 	iomcu_rst: reset-controller {
> >>>>>>>
> >>>>>>>> +		compatible = "hisilicon,hi3660-reset";
> >>>>>>>> +		#reset-cells = <1>;
> >>>>>>>> +		hisi,rst-syscon = <&iomcu>;
> >>>>>>>> +		hisi,reset-bits = <0x20 0x8		/* 0: i2c0 */
> >>>>>>>> +				   0x20 0x10		/* 1: i2c1 */
> >>>>>>>> +				   0x20 0x20		/* 2: i2c2 */
> >>>>>>>> +				   0x20 0x8000000>;	/* 3: i2c6 */
> >>>>>>>> +	};
> >>>>>>> The reset lines are controlled through iomcu bits, is there a reason not
> >>>>>>> to put the iomcu_rst node inside the iomcu node? That way the
> >>>>>>> hisi,rst-syscon property could be removed and the syscon could be
> >>>>>>> retrieved via the reset-controller parent node.
> >>>>>> iomcu is common registers, controls clock and reset, etc.
> >>>>>> So we use syscon, without mapping the registers everywhere.
> >>>>>> It is common case in hisilicon, same in hi6220.
> >>>>>>
> >>>>>> Also the #clock-cells and #reset-cells can not be put in the same node,
> >>>>>> if they are both using probe, since reset_probe will not be called.
> >>>>>>
> >>>>>> So we use hisi,rst-syscon as a general solution.
> >>>>> What I meant is this:
> >>>>>
> >>>>> 	iomcu: iomcu@ffd7e000 {
> >>>>> 		compatible = "hisilicon,hi3660-iomcu", "syscon", "simple-mfd";
> >>>>> 		reg = <0x0 0xffd7e000 0x0 0x1000>;
> >>>> #clock-cells = <1>;
> >>>>
> >>>> In my test, if there add #clock-cells = <1>, reset_probe will not be
> >>>> called any more.
> >>>> Since clk_probe is called first.
> >>>> No matter iomcu_rst is child node or not.
> >>> I don't understand this, does the clock driver bind to the iomcu node
> >>> using CLK_OF_DECLARE_DRIVER(..., "hisilicon,hi3660-iomcu", ...)?
> >> This method:CLK_OF_DECLARE_DRIVER is not prefered in clock,
> >> and we have to use probe instead, to make all driver build as modules as
> >> possible.
> >>
> >> For example hi3660.
> >> static struct platform_driver hi3660_clk_driver = {
> >>           .probe          = hi3660_clk_probe,
> >>           .driver         = {
> >>                   .name   = "hi3660-clk",
> >>                   .of_match_table = hi3660_clk_match_table,
> >>           },
> >> };
> > hi3660_clk_match_table contains the "hisilicon,hi3660-iomcu" compatible?
> > If so, you could call
> > 	of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> > from hi3660_clk_probe instead of using "simple-mfd" to probe the iomcu
> > node's children.
> 
> Not using simple-mfd:
> 
> Like
> static const struct of_device_id hi3660_clk_match_table[] = {
>          { .compatible = "hisilicon,hi3660-iomcu", },
>          { }
> };
> MODULE_DEVICE_TABLE(of, hi3660_clk_match_table);
> 
> static int hi3660_clk_probe(struct platform_device *pdev)
> {
>          struct device *dev = &pdev->dev;
>          struct device_node *np = pdev->dev.of_node;
>          const struct of_device_id *of_id;
>          enum hi3660_clk_type type;
> 
>          of_id = of_match_device(hi3660_clk_match_table, dev);
>          if (!of_id)
>                  return -EINVAL;
> ~
> }
> 
> If put iomcu_rst as child node, and set #clock-cells = <1> to iomcu,
> then hi3660_clk_probe is called, hi3660_reset_probe will not be called.

For hi3660_reset_probe to be called, you'll have to call
of_platform_populate to probe the hi3660-iomcu children in this case.

> So using "hisi,rst-syscon" as pointer does not have the issue.

I understand that, it still sounds to me like you are organizing the
device tree around limitations of the current code. Instead the device
tree should be organized to best describe the hardware, and the code
should be adapted to support that.

Of course, if you use the flat DT layout everywhere else, I won't try to
block the reset driver because of this issue. I'm just saying nested
nodes in the DT would better describe the real control flow.

regards
Philipp


_______________________________________________
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 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
From: Richard Cochran @ 2016-11-30  9:56 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
	Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok
In-Reply-To: <20161128230428.6872-3-grygorii.strashko-l0cyMroinI0@public.gmane.org>

On Mon, Nov 28, 2016 at 05:04:24PM -0600, Grygorii Strashko wrote:
> Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet
> Switch Subsystems, can control an external multiplexer that selects
> one of up to 32 clocks for time sync reference (RFTCLK). This feature
> can be configured through CPTS_RFTCLK_SEL register (offset: x08).
> 
> Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present,
> will specify CPTS reference clock. The cpts_rftclk_sel should be
> omitted in DT if HW doesn't support this feature. The external fixed
> rate clocks can be defined in board files as "fixed-clock".

Can't you implement this using the clock tree, rather than an ad-hoc
DT property?

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Laurent Pinchart @ 2016-11-30  9:52 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
	Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161130102757.9eec1f7f3377d0f4787e3829-GANU6spQydw@public.gmane.org>

Hi Jean-François,

On Wednesday 30 Nov 2016 10:27:57 Jean-Francois Moine wrote:
> On Wed, 30 Nov 2016 10:20:21 +0200 Laurent Pinchart wrote:
> >> Well, I don't see what this connector can be.
> >> May you give me a DT example?
> > 
> > Sure.
> > 
> > arch/arm/boot/dts/r8a7791-koelsch.dts
> > 
> >         /* HDMI encoder */
> >         
> >         hdmi@39 {
> >                 compatible = "adi,adv7511w";
> >                 reg = <0x39>;
> >                 interrupt-parent = <&gpio3>;
> >                 interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> >                 
> >                 adi,input-depth = <8>;
> >                 adi,input-colorspace = "rgb";
> >                 adi,input-clock = "1x";
> >                 adi,input-style = <1>;
> >                 adi,input-justification = "evenly";
> >                 
> >                 ports {
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         
> >                         port@0 {
> >                                 reg = <0>;
> >                                 adv7511_in: endpoint {
> >                                         remote-endpoint = <&du_out_rgb>;
> >                                 };
> >                         };
> >                         
> >                         port@1 {
> >                                 reg = <1>;
> >                                 adv7511_out: endpoint {
> >                                         remote-endpoint = <&hdmi_con>;
> >                                 };
> >                         };
> >                 };
> >         
> >         };
> >         
> >         /* HDMI connector */
> >         
> >         hdmi-out {
> >                 compatible = "hdmi-connector";
> >                 type = "a";
> >                 
> >                 port {
> >                         hdmi_con: endpoint {
> >                                 remote-endpoint = <&adv7511_out>;
> >                         };
> >                 };
> >         };
> 
> Hi Laurent,
> 
> Sorry for I don't see the interest:
> - it is obvious that the HDMI connector is a 'hdmi-connector'!

It still has to be told to the drivers, they don't know how to identify a 
connector by looking at it :-)

> - the physical connector type may be changed on any board by a soldering
>   iron or a connector to connector cable.

Which is also true for any other component on the board. DT (and for that 
matter any firmware description of the platform) isn't soldering-proof.

> - what does the software do with the connector type?

That's up to the software to decide, the DT bindings should describe the 
hardware in the most accurate and usable way for the OS as possible. One of my 
longer term goals is to add connector drivers to handle DDC and HPD when 
they're not handled by the encoder (they are in the above example).

If the DDC was connected to a general-purpose I2C bus of the SoC, and the HPD 
to a GPIO, we would have

	hdmi-out {
		compatible = "hdmi-connector";
		type = "a";
		/* I2C bus and GPIO references are made up for the example */
		ddc-i2c-bus = <&i2c4>;
		hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>

		port {
			hdmi_con: endpoint {
				remote-endpoint = <&adv7511_out>;
			};
		};
	};

and both HPD and EDID reading should be handled by the connector driver.

> - why not to put the connector information in the HDMI device?

Because the connector is separate from the encoder. It's not uncommon 
(depending on the encoder type) to have the encoder output connected to a non-
connector entity such as another chained encoder.

For example most LVDS encoders are connected to a panel, but I have a board 
with the following encoders chain.

CRTC -- parallel RGB --> on-SoC LVDS encoder -- LVDS --> on-board LVDS decoder 
-- parallel RGB --> HDMI encoder -- HDMI --> HDMI connector

I can't support that if the LVDS encoder driver hardcodes the assumption that 
the encoder output is connected to a panel. This kind of usage might be less 
common for HDMI but is certainly not inconceivable.

> And, if I follow you, the graph of ports could also be used to describe
> the way the various parts of the SoCs are powered, to describe the pin
> connections, to describe the USB connectors, to describe the board
> internal hubs and bridges...

It should be used where applicable, it's not meant as the only possible 
hardware description for all pieces of the system.

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* [PATCH v2] arm64: dts: zx: add zx296718's topcrm node
From: Baoyou Xie @ 2016-11-30  9:50 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, jun.nie-QSEj5FYQhm4dnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	baoyou.xie-QSEj5FYQhm4dnm+yROfE0A,
	xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
	chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
	wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A

Enable topcrm clock node for zx296718, which is used for
CPU's frequency change.

Furthermore, this patch adds the CPU clock phandle in CPU's node
and uses operating-points-v2 to register operating points.

So it can be used by cpufreq-dt driver.

Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/boot/dts/zte/zx296718.dtsi | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index 6b239a3..992158a 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -44,6 +44,7 @@
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clock/zx296718-clock.h>
 
 / {
 	compatible = "zte,zx296718";
@@ -81,6 +82,8 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x0>;
 			enable-method = "psci";
+			clocks = <&topcrm A53_GATE>;
+			operating-points-v2 = <&cluster0_opp>;
 		};
 
 		cpu1: cpu@1 {
@@ -88,6 +91,7 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x1>;
 			enable-method = "psci";
+			operating-points-v2 = <&cluster0_opp>;
 		};
 
 		cpu2: cpu@2 {
@@ -95,6 +99,7 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x2>;
 			enable-method = "psci";
+			operating-points-v2 = <&cluster0_opp>;
 		};
 
 		cpu3: cpu@3 {
@@ -102,6 +107,38 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x3>;
 			enable-method = "psci";
+			operating-points-v2 = <&cluster0_opp>;
+		};
+	};
+
+	cluster0_opp: opp_table0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp@500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+			opp-microvolt = <857000>;
+			clock-latency-ns = <500000>;
+		};
+		opp@648000000 {
+			opp-hz = /bits/ 64 <648000000>;
+			opp-microvolt = <857000>;
+			clock-latency-ns = <500000>;
+		};
+		opp@800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <882000>;
+			clock-latency-ns = <500000>;
+		};
+		opp@1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			opp-microvolt = <892000>;
+			clock-latency-ns = <500000>;
+		};
+		opp@1188000000 {
+			opp-hz = /bits/ 64 <1188000000>;
+			opp-microvolt = <1009000>;
+			clock-latency-ns = <500000>;
 		};
 	};
 
@@ -279,6 +316,12 @@
 			dma-requests = <32>;
 		};
 
+		topcrm: clock-controller@1461000 {
+			compatible = "zte,zx296718-topcrm";
+			reg = <0x01461000 0x1000>;
+			#clock-cells = <1>;
+		};
+
 		sysctrl: sysctrl@1463000 {
 			compatible = "zte,zx296718-sysctrl", "syscon";
 			reg = <0x1463000 0x1000>;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH net-next v4 0/4] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-30  9:47 UTC (permalink / raw)
  To: Florian Fainelli, netdev, devicetree
  Cc: Andrew Lunn, Alexandre TORGUE, Neil Armstrong,
	Martin Blumenstingl, Kevin Hilman, linux-kernel, Yegor Yefremov,
	Julia Lawall, Andre Roth, linux-amlogic, Carlo Caione,
	Giuseppe Cavallaro, Andreas Färber, linux-arm-kernel
In-Reply-To: <049b1efc-3bad-92e0-45ef-0563dc5d81de@gmail.com>

On Mon, 2016-11-28 at 09:54 -0800, Florian Fainelli wrote:
> On 11/28/2016 07:50 AM, Jerome Brunet wrote:
> > 
> > This patchset fixes an issue with the OdroidC2 board (DWMAC +
> > RTL8211F).
> > The platform seems to enter LPI on the Rx path too often while
> > performing
> > relatively high TX transfer. This eventually break the link (both
> > Tx and
> > Rx), and require to bring the interface down and up again to get
> > the Rx
> > path working again.
> > 
> > The root cause of this issue is not fully understood yet but
> > disabling EEE
> > advertisement on the PHY prevent this feature to be negotiated.
> > With this change, the link is stable and reliable, with the
> > expected
> > throughput performance.
> > 
> > The patchset adds options in the generic phy driver to disable EEE
> > advertisement, through device tree. The way it is done is very
> > similar
> > to the handling of the max-speed property.
> > 
> > Patch 4 is provided here for testing purpose only. Please don't
> > merge
> > patch 4, this change will go through the amlogic's tree.
> 
> Sorry, but I really don't like the route this is going, and I should
> have made myself clearer before on that, I really think utilizing a
> PHY
> fixup is more appropriate here than an extremely generic DT property.
> The fixup code can be in the affected PHY driver, or it can be
> somewhere
> else, your call. There is no shortage of option on how to implement
> it,
> and this would be something easy to enable/disable for known good
> configurations (ala PCI/USB fixups).
> 
> If we start supporting generic "enable", "disable" type of properties
> with values that map directly to register definitions of the HW, we
> leave too much room for these properties to be utilized to implement
> a
> specific policy, and this is not acceptable.

Florian, 

I agree that DT should not be used to setup a policy, but to describe
what the HW is.

I tried to implement it the way you suggested, using phy fixup, too see
what it looks like.
There is 2 places in the code that seems (remotely) linked to the
issue: 
- meson8b_dwmac driver : if the mac, regardless of the board/platform,
 could not tolerate to have EEE activated, it would make sense to have
the fixup here. It can provide a C callback for such case.
- realtek phy driver: philosophy is kind of the same

To be clear, it is doable and it works that way, but I don't think
embedding this directly in the code is the right way to do it. It seems
we are hiding an information specific about the board inside a generic
driver.

We have several amlogic's design with the same MAC, sometimes with the
same PHY, which have no problem with EEE at all. The issue is really
about the board design.

What I propose is not an enable/disable configuration switch, but to
clearly state that a particular mode of operation is broken. Like the
"max-speed" property, it setup a restriction. IMO, this is a
description of what the HW is and is capable of, and as such it should
be part of the DT.

Yes the property directly map to a register, but it does let you
directly manipulate it (you can't pass the value you want to write in
the register). Having it this way just makes the code simple on both
ends (user and driver).

Yes people could start abusing this to setup policy. In the end, it is
our responsibility, as community, to make sure APIs are used in a
proper way, and not let it be used that way.

I'm open to suggestion on how improve the solution, maybe something
which could bring more confidence that property won't be misused.

Jerome



_______________________________________________
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 1/6] net: ethernet: ti: netcp: add support of cpts
From: Richard Cochran @ 2016-11-30  9:44 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
	linux-omap, Rob Herring, devicetree, Murali Karicheri,
	Wingman Kwok
In-Reply-To: <20161128230428.6872-2-grygorii.strashko@ti.com>

On Mon, Nov 28, 2016 at 05:04:23PM -0600, Grygorii Strashko wrote:
> @@ -678,6 +744,9 @@ struct gbe_priv {
>  	int				num_et_stats;
>  	/*  Lock for updating the hwstats */
>  	spinlock_t			hw_stats_lock;
> +
> +	int                             cpts_registered;

The usage of this counter is racy.

> +	struct cpts                     *cpts;
>  };

This ++ and -- business ...

> +static void gbe_register_cpts(struct gbe_priv *gbe_dev)
> +{
> +	if (!gbe_dev->cpts)
> +		return;
> +
> +	if (gbe_dev->cpts_registered > 0)
> +		goto done;
> +
> +	if (cpts_register(gbe_dev->cpts)) {
> +		dev_err(gbe_dev->dev, "error registering cpts device\n");
> +		return;
> +	}
> +
> +done:
> +	++gbe_dev->cpts_registered;
> +}
> +
> +static void gbe_unregister_cpts(struct gbe_priv *gbe_dev)
> +{
> +	if (!gbe_dev->cpts || (gbe_dev->cpts_registered <= 0))
> +		return;
> +
> +	if (--gbe_dev->cpts_registered)
> +		return;
> +
> +	cpts_unregister(gbe_dev->cpts);
> +}

is invoked from your open() and close() methods, but those methods
are not serialized among multiple ports.

Thanks,
Richard

^ permalink raw reply

* [PATCH v2 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

This commit enables the device node spi0 on the official development
board for the Marvell Armada 3700. It also adds sub-node for the 128Mb
SPI-NOR present on the board.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..0c4eb98 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -67,6 +67,36 @@
 	status = "okay";
 };
 
+&spi0 {
+	status = "okay";
+
+	m25p80@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <108000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			partition@0 {
+				label = "bootloader";
+				reg = <0x0 0x200000>;
+			};
+			partition@200000 {
+				label = "U-boot Env";
+				reg = <0x200000 0x10000>;
+			};
+			partition@210000 {
+				label = "Linux";
+				reg = <0x210000 0xDF0000>;
+			};
+		};
+	};
+};
+
 /* Exported on the micro USB connector CON32 through an FTDI */
 &uart0 {
 	status = "okay";
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Armada 3700 SoC has an SPI Controller, this commit adds the definition
of the SPI device node at the SoC level.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

Changes in v2:
 - Removed properties max-frequency and clock-frequency, it is no
   longer required and not used by the DT-bindings.

 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9bd587..63c2002 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -98,6 +98,17 @@
 			/* 32M internal register @ 0xd000_0000 */
 			ranges = <0x0 0x0 0xd0000000 0x2000000>;
 
+			spi0: spi@10600 {
+				compatible = "marvell,armada-3700-spi";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x10600 0x5d>;
+				clocks = <&nb_periph_clk 7>;
+				interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+				num-cs = <4>;
+				status = "disabled";
+			};
+
 			uart0: serial@12000 {
 				compatible = "marvell,armada-3700-uart";
 				reg = <0x12000 0x400>;
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 3/5] dt-bindings: spi: Add documentation for the Armada 3700 SPI Controller
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: Mark Brown, linux-spi
  Cc: Mark Rutland, Andrew Lunn, Jason Cooper, Pawel Moll, devicetree,
	Ian Campbell, Nadav Haklai, Rob Herring, Kumar Gala,
	Gregory Clement, xigu, dingwei, Thomas Petazzoni,
	linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161130094351.2748-1-romain.perier@free-electrons.com>

This adds the devicetree bindings documentation for the SPI controller
present in the Marvell Armada 3700 SoCs.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 .../devicetree/bindings/spi/spi-armada-3700.txt    | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-armada-3700.txt b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
new file mode 100644
index 0000000..1564aa8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
@@ -0,0 +1,25 @@
+* Marvell Armada 3700 SPI Controller
+
+Required Properties:
+
+- compatible: should be "marvell,armada-3700-spi"
+- reg: physical base address of the controller and length of memory mapped
+       region.
+- interrupts: The interrupt number. The interrupt specifier format depends on
+	      the interrupt controller and of its driver.
+- clocks: Must contain the clock source, usually from the North Bridge clocks.
+- num-cs: The number of chip selects that is supported by this SPI Controller
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Example:
+
+	spi0: spi@10600 {
+		compatible = "marvell,armada-3700-spi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10600 0x5d>;
+		clocks = <&nb_perih_clk 7>;
+		interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+		num-cs = <4>;
+	};
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 2/5] spi: armada-3700: Add support for the FIFO mode
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

In FIFO mode, dedicated registers are used to store the instruction,
the address, the read mode and the data. Write and Read FIFO are used
to store the outcoming or incoming data. The CPU no longer has to assert
each byte. The data FIFOs are accessible via DMA or by the CPU.

This commit adds support for the FIFO mode with the CPU.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

Changes in v2:
 - Removed a3700_spi_bytelen_set from the setup function, it was accidentally
   let here and not required, as it is configured in the prepare callback now
   (defaults to 4 for fifo mode). It solves unrecognized spi-nor flash memory
   detection with jedec.

 drivers/spi/spi-armada-3700.c | 409 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 399 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index cc9e1b2..72f1818 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -99,19 +99,28 @@
 /* A3700_SPI_IF_TIME_REG */
 #define A3700_SPI_CLK_CAPT_EDGE		BIT(7)
 
+/* Flags and macros for struct a3700_spi */
+#define HAS_FIFO			BIT(0)
+#define A3700_INSTR_CNT			1
+#define A3700_ADDR_CNT			3
+#define A3700_DUMMY_CNT			1
+
 struct a3700_spi {
 	struct spi_master *master;
 	void __iomem *base;
 	struct clk *clk;
 	unsigned int irq;
 	unsigned int flags;
-	bool last_xfer;
+	bool xmit_data;
 	const u8 *tx_buf;
 	u8 *rx_buf;
 	size_t buf_len;
 	u8 byte_len;
 	u32 wait_mask;
 	struct completion done;
+	u32 addr_cnt;
+	u32 instr_cnt;
+	size_t hdr_cnt;
 };
 
 static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
@@ -180,12 +189,15 @@ static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
 	return 0;
 }
 
-static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+static void a3700_spi_fifo_mode_set(struct a3700_spi *a3700_spi)
 {
 	u32 val;
 
 	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
-	val &= ~A3700_SPI_FIFO_MODE;
+	if (a3700_spi->flags & HAS_FIFO)
+		val |= A3700_SPI_FIFO_MODE;
+	else
+		val &= ~A3700_SPI_FIFO_MODE;
 	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
 }
 
@@ -255,11 +267,30 @@ static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
 	a3700_spi->byte_len = len;
 }
 
+static int a3700_spi_fifo_flush(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_FIFO_FLUSH;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_FIFO_FLUSH))
+			return 0;
+		udelay(1);
+	}
+
+	return -ETIMEDOUT;
+}
+
 static int a3700_spi_init(struct a3700_spi *a3700_spi)
 {
 	struct spi_master *master = a3700_spi->master;
 	u32 val;
-	int i;
+	int i, ret = 0;
 
 	/* Reset SPI unit */
 	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
@@ -278,10 +309,8 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
 	for (i = 0; i < master->num_chipselect; i++)
 		a3700_spi_deactivate_cs(a3700_spi, i);
 
-	a3700_spi_pin_mode_set(a3700_spi, 0);
-
-	/* Be sure that FIFO mode is disabled */
-	a3700_spi_fifo_mode_unset(a3700_spi);
+	/* Enable FIFO mode */
+	a3700_spi_fifo_mode_set(a3700_spi);
 
 	/* Set SPI mode */
 	a3700_spi_mode_set(a3700_spi, master->mode_bits);
@@ -294,7 +323,7 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
 	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
 	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
 
-	return 0;
+	return ret;
 }
 
 static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
@@ -380,14 +409,34 @@ static bool a3700_spi_transfer_wait(struct spi_device *spi,
 	return a3700_spi_wait_completion(spi);
 }
 
+static void a3700_spi_fifo_thres_set(struct a3700_spi *a3700_spi,
+				     unsigned int bytes)
+{
+	u32 val;
+
+	if (a3700_spi->flags & HAS_FIFO) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_RFIFO_THRS_BIT);
+		val |= (bytes - 1) << A3700_SPI_RFIFO_THRS_BIT;
+		val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_WFIFO_THRS_BIT);
+		val |= (7 - bytes) << A3700_SPI_WFIFO_THRS_BIT;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	}
+}
+
 static void a3700_spi_transfer_setup(struct spi_device *spi,
 				    struct spi_transfer *xfer)
 {
 	struct a3700_spi *a3700_spi;
+	unsigned int byte_len;
 
 	a3700_spi = spi_master_get_devdata(spi->master);
 
 	a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+
+	byte_len = xfer->bits_per_word >> 3;
+
+	a3700_spi_fifo_thres_set(a3700_spi, byte_len);
 }
 
 static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
@@ -447,6 +496,168 @@ static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
 		a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
 }
 
+static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
+{
+	u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+	u32 val = 0;
+
+	/* Clear the header registers */
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+
+	/* Set header counters */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
+			instr_cnt = a3700_spi->buf_len;
+		} else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
+						  a3700_spi->addr_cnt)) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->buf_len - instr_cnt;
+		} else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->addr_cnt;
+			/* Need to handle the normal write case with 1 byte
+			 * data
+			 */
+			if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
+				dummy_cnt = a3700_spi->buf_len - instr_cnt -
+					    addr_cnt;
+		}
+		val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
+			<< A3700_SPI_INSTR_CNT_BIT);
+		val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+			<< A3700_SPI_ADDR_CNT_BIT);
+		val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
+			<< A3700_SPI_DUMMY_CNT_BIT);
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+	/* Update the buffer length to be transferred */
+	a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
+
+	/* Set Instruction */
+	val = 0;
+	while (instr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
+
+	/* Set Address */
+	val = 0;
+	while (addr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
+}
+
+static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	return (val & A3700_SPI_WFIFO_FULL);
+}
+
+static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+	int i = 0;
+
+	while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
+		val = 0;
+		if (a3700_spi->buf_len >= 4) {
+			val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+			spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->tx_buf += 4;
+		} else {
+			/*
+			 * If the remained buffer length is less than 4-bytes,
+			 * we should pad the write buffer with all ones. So that
+			 * it avoids overwrite the unexpected bytes following
+			 * the last one.
+			 */
+			val = GENMASK(31, 0);
+			while (a3700_spi->buf_len) {
+				val &= ~(0xff << (8 * i));
+				val |= *a3700_spi->tx_buf++ << (8 * i);
+				i++;
+				a3700_spi->buf_len--;
+
+				spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
+					     val);
+			}
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static int a3700_is_rfifo_empty(struct a3700_spi *a3700_spi)
+{
+	u32 val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+
+	return (val & A3700_SPI_RFIFO_EMPTY);
+}
+
+static int a3700_spi_fifo_read(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	while (!a3700_is_rfifo_empty(a3700_spi) && a3700_spi->buf_len) {
+		val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+		if (a3700_spi->buf_len >= 4) {
+			u32 data = le32_to_cpu(val);
+			memcpy(a3700_spi->rx_buf, &data, 4);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->rx_buf += 4;
+		} else {
+			/*
+			 * When remain bytes is not larger than 4, we should
+			 * avoid memory overwriting and just write the left rx
+			 * buffer bytes.
+			 */
+			while (a3700_spi->buf_len) {
+				*a3700_spi->rx_buf = val & 0xff;
+				val >>= 8;
+
+				a3700_spi->buf_len--;
+				a3700_spi->rx_buf++;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void a3700_spi_transfer_abort_fifo(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	a3700_spi_fifo_flush(a3700_spi);
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
 static int a3700_spi_prepare_message(struct spi_master *master,
 				     struct spi_message *message)
 {
@@ -463,12 +674,28 @@ static int a3700_spi_prepare_message(struct spi_master *master,
 	return 0;
 }
 
+static int a3700_spi_prepare_fifo_message(struct spi_master *master,
+					  struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret;
+
+	/* Flush the FIFOs */
+	ret = a3700_spi_fifo_flush(a3700_spi);
+	if (ret)
+		return ret;
+
+	a3700_spi_bytelen_set(a3700_spi, 4);
+
+	return 0;
+}
+
 static int a3700_spi_transfer_one(struct spi_master *master,
 				  struct spi_device *spi,
 				  struct spi_transfer *xfer)
 {
 	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
-	int ret = 0;
+	int ret;
 
 	a3700_spi_transfer_setup(spi, xfer);
 
@@ -505,6 +732,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
 	return ret;
 }
 
+static int a3700_spi_fifo_transfer_one(struct spi_master *master,
+				       struct spi_device *spi,
+				       struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret = 0, timeout = A3700_SPI_TIMEOUT;
+	unsigned int nbits = 0;
+	u32 val;
+
+	a3700_spi_transfer_setup(spi, xfer);
+
+	a3700_spi->tx_buf  = xfer->tx_buf;
+	a3700_spi->rx_buf  = xfer->rx_buf;
+	a3700_spi->buf_len = xfer->len;
+
+	/* SPI transfer headers */
+	a3700_spi_header_set(a3700_spi);
+
+	if (xfer->tx_buf)
+		nbits = xfer->tx_nbits;
+	else if (xfer->rx_buf)
+		nbits = xfer->rx_nbits;
+
+	a3700_spi_pin_mode_set(a3700_spi, nbits);
+
+	if (xfer->rx_buf) {
+		/* Set read data length */
+		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
+			     a3700_spi->buf_len);
+		/* Start READ transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val &= ~A3700_SPI_RW_EN;
+		val |= A3700_SPI_XFER_START;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	} else if (xfer->tx_buf) {
+		/* Start Write transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= (A3700_SPI_XFER_START | A3700_SPI_RW_EN);
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+		/*
+		 * If there are data to be written to the SPI device, xmit_data
+		 * flag is set true; otherwise the instruction in SPI_INSTR does
+		 * not require data to be written to the SPI device, then
+		 * xmit_data flag is set false.
+		 */
+		a3700_spi->xmit_data = (a3700_spi->buf_len != 0);
+	}
+
+	while (a3700_spi->buf_len) {
+		if (a3700_spi->tx_buf) {
+			/* Wait wfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait wfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Fill up the wfifo */
+			ret = a3700_spi_fifo_write(a3700_spi);
+			if (ret)
+				goto error;
+		} else if (a3700_spi->rx_buf) {
+			/* Wait rfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_RFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait rfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Drain out the rfifo */
+			ret = a3700_spi_fifo_read(a3700_spi);
+			if (ret)
+				goto error;
+		}
+	}
+
+	/*
+	 * Stop a write transfer in fifo mode:
+	 *	- wait all the bytes in wfifo to be shifted out
+	 *	 - set XFER_STOP bit
+	 *	- wait XFER_START bit clear
+	 *	- clear XFER_STOP bit
+	 * Stop a read transfer in fifo mode:
+	 *	- the hardware is to reset the XFER_START bit
+	 *	   after the number of bytes indicated in DIN_CNT
+	 *	   register
+	 *	- just wait XFER_START bit clear
+	 */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->xmit_data) {
+			/*
+			 * If there are data written to the SPI device, wait
+			 * until SPI_WFIFO_EMPTY is 1 to wait for all data to
+			 * transfer out of write FIFO.
+			 */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_EMPTY)) {
+				dev_err(&spi->dev, "wait wfifo empty timed out\n");
+				return -ETIMEDOUT;
+			}
+		} else {
+			/*
+			 * If the instruction in SPI_INSTR does not require data
+			 * to be written to the SPI device, wait until SPI_RDY
+			 * is 1 for the SPI interface to be in idle.
+			 */
+			if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+				dev_err(&spi->dev, "wait xfer ready timed out\n");
+				return -ETIMEDOUT;
+			}
+		}
+
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= A3700_SPI_XFER_STOP;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	}
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	if (timeout == 0) {
+		dev_err(&spi->dev, "wait transfer start clear timed out\n");
+		ret = -ETIMEDOUT;
+		goto error;
+	}
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	goto out;
+
+error:
+	a3700_spi_transfer_abort_fifo(a3700_spi);
+out:
+	spi_finalize_current_transfer(master);
+
+	return ret;
+}
+
 static int a3700_spi_unprepare_message(struct spi_master *master,
 				       struct spi_message *message)
 {
@@ -592,6 +964,23 @@ static int a3700_spi_probe(struct platform_device *pdev)
 		goto error;
 	}
 
+	if (of_device_is_compatible(of_node, "marvell,armada-3700-spi")) {
+		master->prepare_message =  a3700_spi_prepare_fifo_message;
+		master->transfer_one = a3700_spi_fifo_transfer_one;
+
+		spi->flags |= HAS_FIFO;
+		spi->instr_cnt = A3700_INSTR_CNT;
+		spi->addr_cnt = A3700_ADDR_CNT;
+		spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
+			       A3700_DUMMY_CNT;
+		master->mode_bits |= (SPI_RX_DUAL | SPI_RX_DUAL |
+				      SPI_RX_QUAD | SPI_TX_QUAD);
+	} else {
+		master->prepare_message =  a3700_spi_prepare_message;
+		master->transfer_one = a3700_spi_transfer_one;
+		master->unprepare_message = a3700_spi_unprepare_message;
+	}
+
 	ret = a3700_spi_init(spi);
 	if (ret)
 		goto error_clk;
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Marvell Armada 3700 SoC comprises an SPI Controller. This Controller
supports up to 4 SPI slave devices, with dedicated chip selects, supports
SPI mode 0/1/2 and 3, CPIO or Fifo mode with DMA transfers and different
SPI transfer mode (Single, Dual or Quad).

This commit adds basic driver support for CPIO mode and single SPI
transfer. In this mode, the CPU asserts cs, outputs or inputs data from
the current SPI device. Data transfers are copied by 1 or 4 bytes using
the SPI registers.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/spi/Kconfig           |   7 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-armada-3700.c | 651 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 659 insertions(+)
 create mode 100644 drivers/spi/spi-armada-3700.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..6ade1ca 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -67,6 +67,13 @@ config SPI_ATH79
 	  This enables support for the SPI controller present on the
 	  Atheros AR71XX/AR724X/AR913X SoCs.
 
+config SPI_ARMADA_3700
+	tristate "Marvell Armada 3700 SPI Controller"
+	depends on ARCH_MVEBU && OF
+	help
+	  This enables support for the SPI controller present on the
+	  Marvell Armada 3700 SoCs.
+
 config SPI_ATMEL
 	tristate "Atmel SPI Controller"
 	depends on HAS_DMA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index aa939d9..140ca45 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 
 # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
+obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
new file mode 100644
index 0000000..cc9e1b2
--- /dev/null
+++ b/drivers/spi/spi-armada-3700.c
@@ -0,0 +1,651 @@
+/*
+ * Marvell Armada-3700 SPI controller driver
+ *
+ * Copyright (C) 2016 Marvell Ltd.
+ *
+ * Author: Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
+ * Author: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME			"armada_3700_spi"
+
+#define A3700_SPI_TIMEOUT		10
+
+/* SPI Register Offest */
+#define A3700_SPI_IF_CTRL_REG		0x00
+#define A3700_SPI_IF_CFG_REG		0x04
+#define A3700_SPI_DATA_OUT_REG		0x08
+#define A3700_SPI_DATA_IN_REG		0x0C
+#define A3700_SPI_IF_INST_REG		0x10
+#define A3700_SPI_IF_ADDR_REG		0x14
+#define A3700_SPI_IF_RMODE_REG		0x18
+#define A3700_SPI_IF_HDR_CNT_REG	0x1C
+#define A3700_SPI_IF_DIN_CNT_REG	0x20
+#define A3700_SPI_IF_TIME_REG		0x24
+#define A3700_SPI_INT_STAT_REG		0x28
+#define A3700_SPI_INT_MASK_REG		0x2C
+
+/* A3700_SPI_IF_CTRL_REG */
+#define A3700_SPI_EN			BIT(16)
+#define A3700_SPI_ADDR_NOT_CONFIG	BIT(12)
+#define A3700_SPI_WFIFO_OVERFLOW	BIT(11)
+#define A3700_SPI_WFIFO_UNDERFLOW	BIT(10)
+#define A3700_SPI_RFIFO_OVERFLOW	BIT(9)
+#define A3700_SPI_RFIFO_UNDERFLOW	BIT(8)
+#define A3700_SPI_WFIFO_FULL		BIT(7)
+#define A3700_SPI_WFIFO_EMPTY		BIT(6)
+#define A3700_SPI_RFIFO_FULL		BIT(5)
+#define A3700_SPI_RFIFO_EMPTY		BIT(4)
+#define A3700_SPI_WFIFO_RDY		BIT(3)
+#define A3700_SPI_RFIFO_RDY		BIT(2)
+#define A3700_SPI_XFER_RDY		BIT(1)
+#define A3700_SPI_XFER_DONE		BIT(0)
+
+/* A3700_SPI_IF_CFG_REG */
+#define A3700_SPI_WFIFO_THRS		BIT(28)
+#define A3700_SPI_RFIFO_THRS		BIT(24)
+#define A3700_SPI_AUTO_CS		BIT(20)
+#define A3700_SPI_DMA_RD_EN		BIT(18)
+#define A3700_SPI_FIFO_MODE		BIT(17)
+#define A3700_SPI_SRST			BIT(16)
+#define A3700_SPI_XFER_START		BIT(15)
+#define A3700_SPI_XFER_STOP		BIT(14)
+#define A3700_SPI_INST_PIN		BIT(13)
+#define A3700_SPI_ADDR_PIN		BIT(12)
+#define A3700_SPI_DATA_PIN1		BIT(11)
+#define A3700_SPI_DATA_PIN0		BIT(10)
+#define A3700_SPI_FIFO_FLUSH		BIT(9)
+#define A3700_SPI_RW_EN			BIT(8)
+#define A3700_SPI_CLK_POL		BIT(7)
+#define A3700_SPI_CLK_PHA		BIT(6)
+#define A3700_SPI_BYTE_LEN		BIT(5)
+#define A3700_SPI_CLK_PRESCALE		BIT(0)
+#define A3700_SPI_CLK_PRESCALE_MASK	(0x1f)
+
+#define A3700_SPI_WFIFO_THRS_BIT	28
+#define A3700_SPI_RFIFO_THRS_BIT	24
+#define A3700_SPI_FIFO_THRS_MASK	0x7
+
+#define A3700_SPI_DATA_PIN_MASK		0x3
+
+/* A3700_SPI_IF_HDR_CNT_REG */
+#define A3700_SPI_DUMMY_CNT_BIT		12
+#define A3700_SPI_DUMMY_CNT_MASK	0x7
+#define A3700_SPI_RMODE_CNT_BIT		8
+#define A3700_SPI_RMODE_CNT_MASK	0x3
+#define A3700_SPI_ADDR_CNT_BIT		4
+#define A3700_SPI_ADDR_CNT_MASK		0x7
+#define A3700_SPI_INSTR_CNT_BIT		0
+#define A3700_SPI_INSTR_CNT_MASK	0x3
+
+/* A3700_SPI_IF_TIME_REG */
+#define A3700_SPI_CLK_CAPT_EDGE		BIT(7)
+
+struct a3700_spi {
+	struct spi_master *master;
+	void __iomem *base;
+	struct clk *clk;
+	unsigned int irq;
+	unsigned int flags;
+	bool last_xfer;
+	const u8 *tx_buf;
+	u8 *rx_buf;
+	size_t buf_len;
+	u8 byte_len;
+	u32 wait_mask;
+	struct completion done;
+};
+
+static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
+{
+	return readl(a3700_spi->base + offset);
+}
+
+static void spireg_write(struct a3700_spi *a3700_spi, u32 offset, u32 data)
+{
+	writel(data, a3700_spi->base + offset);
+}
+
+static void a3700_spi_auto_cs_unset(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_AUTO_CS;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_activate_cs(struct a3700_spi *a3700_spi, unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val |= (A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static void a3700_spi_deactivate_cs(struct a3700_spi *a3700_spi,
+				    unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val &= ~(A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
+				  unsigned int pin_mode)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_INST_PIN | A3700_SPI_ADDR_PIN);
+	val &= ~(A3700_SPI_DATA_PIN0 | A3700_SPI_DATA_PIN1);
+
+	switch (pin_mode) {
+	case 1:
+		break;
+	case 2:
+		val |= A3700_SPI_DATA_PIN0;
+		break;
+	case 4:
+		val |= A3700_SPI_DATA_PIN1;
+		break;
+	default:
+		dev_err(&a3700_spi->master->dev, "wrong pin mode %u", pin_mode);
+		return -EINVAL;
+	}
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	return 0;
+}
+
+static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_FIFO_MODE;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_mode_set(struct a3700_spi *a3700_spi,
+			       unsigned int mode_bits)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+
+	if (mode_bits & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+	else
+		val &= ~A3700_SPI_CLK_POL;
+
+	if (mode_bits & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+	else
+		val &= ~A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_clock_set(struct a3700_spi *a3700_spi,
+				unsigned int speed_hz, u16 mode)
+{
+	u32 val;
+	u32 prescale;
+
+	prescale = DIV_ROUND_UP(clk_get_rate(a3700_spi->clk), speed_hz);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val = val & ~A3700_SPI_CLK_PRESCALE_MASK;
+
+	val = val | (prescale & A3700_SPI_CLK_PRESCALE_MASK);
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	if (prescale <= 2) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_TIME_REG);
+		val |= A3700_SPI_CLK_CAPT_EDGE;
+		spireg_write(a3700_spi, A3700_SPI_IF_TIME_REG, val);
+	}
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_CLK_POL | A3700_SPI_CLK_PHA);
+
+	if (mode & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+
+	if (mode & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	if (len == 4)
+		val |= A3700_SPI_BYTE_LEN;
+	else
+		val &= ~A3700_SPI_BYTE_LEN;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	a3700_spi->byte_len = len;
+}
+
+static int a3700_spi_init(struct a3700_spi *a3700_spi)
+{
+	struct spi_master *master = a3700_spi->master;
+	u32 val;
+	int i;
+
+	/* Reset SPI unit */
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	for (i = 0; i < A3700_SPI_TIMEOUT; i++)
+		udelay(1);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	/* Disable AUTO_CS and deactivate all chip-selects */
+	a3700_spi_auto_cs_unset(a3700_spi);
+	for (i = 0; i < master->num_chipselect; i++)
+		a3700_spi_deactivate_cs(a3700_spi, i);
+
+	a3700_spi_pin_mode_set(a3700_spi, 0);
+
+	/* Be sure that FIFO mode is disabled */
+	a3700_spi_fifo_mode_unset(a3700_spi);
+
+	/* Set SPI mode */
+	a3700_spi_mode_set(a3700_spi, master->mode_bits);
+
+	/* Reset counters */
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG, 0);
+
+	/* Mask the interrupts and clear cause bits */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
+
+	return 0;
+}
+
+static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_master *master = dev_id;
+	struct a3700_spi *a3700_spi;
+	u32 cause;
+
+	a3700_spi = spi_master_get_devdata(master);
+
+	/* Get interrupt causes */
+	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
+
+	/* mask and acknowledge the SPI interrupts */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
+
+	/* Wake up the transfer */
+	if (a3700_spi->wait_mask & cause)
+		complete(&a3700_spi->done);
+
+	return IRQ_HANDLED;
+}
+
+static bool a3700_spi_wait_completion(struct spi_device *spi)
+{
+	struct a3700_spi *a3700_spi;
+	unsigned int timeout;
+	unsigned int ctrl_reg;
+	unsigned long timeout_jiffies;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	/* SPI interrupt is edge-triggered, which means an interrupt will
+	 * be generated only when detecting a specific status bit changed
+	 * from '0' to '1'. So when we start waiting for a interrupt, we
+	 * need to check status bit in control reg first, if it is already 1,
+	 * then we do not need to wait for interrupt
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	reinit_completion(&a3700_spi->done);
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG,
+		     a3700_spi->wait_mask);
+
+	timeout_jiffies = msecs_to_jiffies(A3700_SPI_TIMEOUT);
+	timeout = wait_for_completion_timeout(&a3700_spi->done,
+					      timeout_jiffies);
+
+	a3700_spi->wait_mask = 0;
+
+	if (timeout)
+		return true;
+
+	/* there might be the case that right after we checked the
+	 * status bits in this routine and before start to wait for
+	 * interrupt by wait_for_completion_timeout, the interrupt
+	 * happens, to avoid missing it we need to double check
+	 * status bits in control reg, if it is already 1, then
+	 * consider that we have the interrupt successfully and
+	 * return true.
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+
+	return true;
+}
+
+static bool a3700_spi_transfer_wait(struct spi_device *spi,
+				    unsigned int bit_mask)
+{
+	struct a3700_spi *a3700_spi;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+	a3700_spi->wait_mask = bit_mask;
+
+	return a3700_spi_wait_completion(spi);
+}
+
+static void a3700_spi_transfer_setup(struct spi_device *spi,
+				    struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+}
+
+static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
+{
+	u32 val, data;
+
+	if (a3700_spi->buf_len % a3700_spi->byte_len)
+		return -EINVAL;
+
+	/* Read bytes from data in register */
+	val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+
+	if (a3700_spi->byte_len == 4)
+		data = be32_to_cpu(val);
+	else
+		data = val;
+
+	memcpy(a3700_spi->rx_buf, &data, a3700_spi->byte_len);
+
+	a3700_spi->buf_len -= a3700_spi->byte_len;
+	a3700_spi->rx_buf  += a3700_spi->byte_len;
+
+	/* Request next 1 or 4 bytes data */
+	if (a3700_spi->buf_len)
+		spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+	return 0;
+}
+
+static int a3700_spi_write_data(struct a3700_spi *a3700_spi)
+{
+	u32 val = 0;
+
+	if (a3700_spi->buf_len % a3700_spi->byte_len)
+		return -EINVAL;
+
+	/* Write bytes from data out register */
+	if (a3700_spi->byte_len == 4)
+		val = cpu_to_be32(*(u32 *)a3700_spi->tx_buf);
+	else
+		val = a3700_spi->tx_buf[0];
+
+	spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+	a3700_spi->buf_len -= a3700_spi->byte_len;
+	a3700_spi->tx_buf  += a3700_spi->byte_len;
+
+	return 0;
+}
+
+static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(spi->master);
+
+	if (!enable)
+		a3700_spi_activate_cs(a3700_spi, spi->chip_select);
+	else
+		a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
+}
+
+static int a3700_spi_prepare_message(struct spi_master *master,
+				     struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	struct spi_device *spi = message->spi;
+
+	a3700_spi_bytelen_set(a3700_spi, 1);
+
+	if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+		dev_err(&spi->dev, "wait transfer ready timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int a3700_spi_transfer_one(struct spi_master *master,
+				  struct spi_device *spi,
+				  struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret = 0;
+
+	a3700_spi_transfer_setup(spi, xfer);
+
+	a3700_spi->tx_buf  = xfer->tx_buf;
+	a3700_spi->rx_buf  = xfer->rx_buf;
+	a3700_spi->buf_len = xfer->len;
+
+	/* Start READ transfer by writing dummy data to DOUT register */
+	if (xfer->rx_buf)
+		spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+	while (a3700_spi->buf_len) {
+		if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+			dev_err(&spi->dev, "wait transfer ready timed out\n");
+			ret = -ETIMEDOUT;
+			goto err;
+		}
+
+		if (a3700_spi->tx_buf) {
+			ret = a3700_spi_write_data(a3700_spi);
+			if (ret)
+				goto err;
+		}
+
+		if (a3700_spi->rx_buf) {
+			ret = a3700_spi_read_data(a3700_spi);
+			if (ret)
+				goto err;
+		}
+	}
+
+err:
+	spi_finalize_current_transfer(master);
+	return ret;
+}
+
+static int a3700_spi_unprepare_message(struct spi_master *master,
+				       struct spi_message *message)
+{
+	struct spi_device *spi = message->spi;
+
+	if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+		dev_err(&spi->dev, "wait transfer ready timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id a3700_spi_dt_ids[] = {
+	{ .compatible = "marvell,armada-3700-spi", .data = NULL },
+};
+
+MODULE_DEVICE_TABLE(of, a3700_spi_of_match_table);
+
+static int a3700_spi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *of_node = dev->of_node;
+	struct resource *res;
+	struct spi_master *master;
+	struct a3700_spi *spi;
+	u32 num_cs = 0;
+	int ret = 0;
+
+	master = spi_alloc_master(dev, sizeof(*spi));
+	if (!master) {
+		dev_err(dev, "master allocation failed\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	if (of_property_read_u32(of_node, "num-cs", &num_cs)) {
+		dev_err(dev, "could not find num-cs\n");
+		ret = -ENXIO;
+		goto error;
+	}
+
+	master->bus_num = (pdev->id != -1) ? pdev->id : 0;
+	master->dev.of_node = of_node;
+	master->mode_bits = SPI_MODE_3;
+	master->num_chipselect = num_cs;
+	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(32);
+	master->prepare_message =  a3700_spi_prepare_message;
+	master->transfer_one = a3700_spi_transfer_one;
+	master->unprepare_message = a3700_spi_unprepare_message;
+	master->set_cs = a3700_spi_set_cs;
+
+	platform_set_drvdata(pdev, master);
+
+	spi = spi_master_get_devdata(master);
+	memset(spi, 0, sizeof(struct a3700_spi));
+
+	spi->master = master;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	spi->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(spi->base)) {
+		ret = PTR_ERR(spi->base);
+		goto error;
+	}
+
+	spi->irq = platform_get_irq(pdev, 0);
+	if (spi->irq < 0) {
+		dev_err(dev, "could not get irq: %d\n", spi->irq);
+		ret = -ENXIO;
+		goto error;
+	}
+
+	init_completion(&spi->done);
+
+	spi->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(spi->clk)) {
+		dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk));
+		goto error;
+	}
+
+	ret = clk_prepare_enable(spi->clk);
+	if (ret) {
+		dev_err(dev, "could not prepare clk: %d\n", ret);
+		goto error;
+	}
+
+	ret = a3700_spi_init(spi);
+	if (ret)
+		goto error_clk;
+
+	ret = devm_request_irq(dev, spi->irq, a3700_spi_interrupt, 0,
+			       dev_name(dev), master);
+	if (ret) {
+		dev_err(dev, "could not request IRQ: %d\n", ret);
+		goto error_clk;
+	}
+
+	ret = devm_spi_register_master(dev, master);
+	if (ret) {
+		dev_err(dev, "Failed to register master\n");
+		goto error_clk;
+	}
+
+	dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
+		 (unsigned long)res->start, spi->irq);
+
+	return 0;
+
+error_clk:
+	clk_disable_unprepare(spi->clk);
+error:
+	spi_master_put(master);
+out:
+	return ret;
+}
+
+static int a3700_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct a3700_spi *spi = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(spi->clk);
+	spi_master_put(master);
+
+	return 0;
+}
+
+static struct platform_driver a3700_spi_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(a3700_spi_dt_ids),
+	},
+	.probe		= a3700_spi_probe,
+	.remove		= a3700_spi_remove,
+};
+
+module_platform_driver(a3700_spi_driver);
+
+MODULE_DESCRIPTION("Armada-3700 SPI driver");
+MODULE_AUTHOR("Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 0/5] Add support for the Armada 3700 SPI controller
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

The Marvell Armada 3700 SoC includes an SPI controller. This controller
supports up to 4 SPI slave devices, with dedicated chip selects, CPIO or
FIFO mode with DMA or CPU transfers and different SPI transfer modes
(Standard single, Dual or Quad).

This set of patches adds a basic support for the CPIO mode, then it
enables the FIFO mode (CPU-side only, DMA not supported yet). It also
adds the required definitions of the spi nodes to the devicetree.

Romain Perier (5):
  spi: Add basic support for Armada 3700 SPI Controller
  spi: armada-3700: Add support for the FIFO mode
  dt-bindings: spi: Add documentation for the Armada 3700 SPI Controller
  arm64: dts: marvell: Add definition of SPI controller for Armada 3700
  arm64: dts: marvell: Enable spi0 on the board Armada-3720-db

 .../devicetree/bindings/spi/spi-armada-3700.txt    |   25 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |   30 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   13 +
 drivers/spi/Kconfig                                |    7 +
 drivers/spi/Makefile                               |    1 +
 drivers/spi/spi-armada-3700.c                      | 1040 ++++++++++++++++++++
 6 files changed, 1116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
 create mode 100644 drivers/spi/spi-armada-3700.c

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH] ARM: dts: sun8i: add simplefb node for H3
From: Jean-Francois Moine @ 2016-11-30  9:35 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Icenowy Zheng, Chen-Yu Tsai, Jernej Skrabec,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161129215932.kmk6qodthicadpyb@lukather>

On Tue, 29 Nov 2016 22:59:32 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> > > I'm still not sure which pipeline should I use.
> > > 
> > > And, it seems that HDMI Slow Clock is not needed?
> > > 
> > > (seems that it's only for EDID, but simplefb won't use EDID)
> > 
> > So, I don't see how this may work.
> > How can the u-boot know the resolutions of the HDMI display device?
> > 
> > In other words: I have a new H3 board with the last u-boot and kernel.
> > I plug my (rather old or brand new) HDMI display device.
> > After powering on the system, I hope to get something on the screen.
> > How?
> 
> If it works like the driver for the first display engine in U-Boot, it
> will use the preferred mode reported by the EDID, and will fallback to
> 1024x768 if it cannot access it.

Icenowy wrote: "simplefb won't use EDID"

Then, if it is like in the kernel, the 1024x768 mode is VGA. It does
not work with HDMI (different timings).

> Maybe it would be worth exchanging on the EDID code that has been done
> for the u-boot driver too, so that it can be fixed in your driver.

The u-boot got my code, and, up to now, I could not fix the random or
permanent failures of EDID reading in some boards.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: Overlays and boolean properties
From: Phil Elwell @ 2016-11-30  9:29 UTC (permalink / raw)
  To: David Gibson, Pantelis Antoniou
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Devicetree Compiler
In-Reply-To: <20161130034733.GH19891@umbus>

Hi David,

On 30/11/2016 03:47, David Gibson wrote:
> On Tue, Nov 29, 2016 at 03:10:40PM +0200, Pantelis Antoniou wrote:
>> Hi Phil,
>>
>>> On Nov 29, 2016, at 15:06 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>>>
>>> Boolean properties are defined as being properties with no content, that
>>> are true if present and false if absent. They pose a problem for DT
>>> overlays, since the proposed (and widely used) overlay mechanism does
>>> not allow for properties (or nodes) to be deleted; overlays can only
>>> make a false property true, so boolean properties are effectively
>>> monostable - once true they become immutable.
>>>
>>> The standard DT syntax includes /delete-property/ and /delete-node/
>>> directives that do what you would expect from their names, but that
>>> facility is not available to overlays. There is no FDT node that
>>> represents the deletion - the directives are acted on immediately
> Uh.. only partially true.  They're acted on during the compile run,
> but not during the parse.  dtc does have an internal representation of
> node or property deletions that gets resolved when we combine the
> fragments in the source file.
>>> - so
>>> we would need some extra markup - say __delete_property__ and
>>> __delete_node__ - to hold the names of items to be deleted.
> So, in principle this wouldn't be that hard - we'd just need to
> translate dtc's internal representation into a representation in the
> dtb.  That could be done with special properties, or with new opcodes
> at the dtb encoding level.
>
>>> Before I take this further, does anybody have any thoughts on the idea?
> So.. the first question, is do we have a pressing use case for this?
> dtbos can (apart from this) alter anything in a base tree, but doing
> so isn't often a good idea.  Usually they'll just add new nodes and
> properties.

This is more of a real world example than a pressing use case, since the
number of people affected is small and there is workaround, albeit an
ugly one.

The Raspberry Pi SoCs have two SD interfaces - one SDIO-capable and one
not. On the Pi3B the SDIO-capable interface is used to drive the WiFi
interface. This WiFi interface doesn't like being continuously prodded
to see if it is still there, so the base DTB sets the "non-removable"
boolean property. Not all Pi3 users want WiFi, preferring instead to
wire up an MMC device or second SD slot via the expansion header. A DT
overlay allows them to modify the pin functions to achieve this, but the
overlay can't (easily) remove the aptly-named "non-removable". The ugly
workaround is to disable the original MMC node and create a clone with a
different name and without the unwanted property.

>> The original patchset did support removing properties (by prefixing them with -).
>>
>> I can revive that if we have consensus about the format/method.
> On the whole, I'd prefer not to see extensions of the existing overlay
> format - instead I'd like to see focus on a new and better thought out
> connector format.
Will this connector mechanism allow a populated DT to be modified, or
would you always have to start with an "empty" board and only add that
which you want? I like the flexibility to be able to perform arbitrary
DT modifications (except deletions) - it has been very useful for
testing and performing staggered rollouts of new functionality.

Phil

^ 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