Devicetree
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: allwinner: a64: Drop PMU node
From: Vasily Khoruzhick @ 2019-08-06 14:01 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
	linux-arm-kernel, devicetree
  Cc: Jared D . McNeill, Harald Geyer

Looks like PMU in A64 is broken, it generates no interrupts at all and
as result 'perf top' shows no events.

Tested on Pine64-LTS.

Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
Cc: Harald Geyer <harald@ccbib.org>
Cc: Jared D. McNeill <jmcneill@NetBSD.org>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 9cc9bdde81ac..cd92f546c483 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -142,15 +142,6 @@
 		clock-output-names = "ext-osc32k";
 	};
 
-	pmu {
-		compatible = "arm,cortex-a53-pmu";
-		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
-	};
-
 	psci {
 		compatible = "arm,psci-0.2";
 		method = "smc";
-- 
2.22.0

^ permalink raw reply related

* [PATCH 2/4 v3] drm/panel: simple: Add TI nspire panel bindings
From: Linus Walleij @ 2019-08-06 13:54 UTC (permalink / raw)
  To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul
  Cc: devicetree, linux-arm-kernel

Add bindings for the TI NSPIRE simple display panels.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChanegLog v2->v3:
- Switch to GPL-2.0-only OR BSD-2-Clause license
- Use a simple enum for the compatible
- Use the new nifty panel-common.yaml, tested on
  linux-next
ChangeLog v1->v2:
- New patch as bindings are required
- Let's use YAML
---
 .../bindings/display/panel/ti,nspire.yaml     | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/ti,nspire.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/ti,nspire.yaml b/Documentation/devicetree/bindings/display/panel/ti,nspire.yaml
new file mode 100644
index 000000000000..5c5a3b519e31
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ti,nspire.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/ti,nspire.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments NSPIRE Display Panels
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ti,nspire-cx-lcd-panel
+      - ti,nspire-classic-lcd-panel
+  port: true
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    panel {
+        compatible = "ti,nspire-cx-lcd-panel";
+        port {
+            panel_in: endpoint {
+                remote-endpoint = <&pads>;
+            };
+        };
+    };
-- 
2.21.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Leonard Crestez @ 2019-08-06 13:41 UTC (permalink / raw)
  To: Artur Świgoń
  Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	krzk@kernel.org, cw00.choi@samsung.com, myungjoo.ham@samsung.com,
	inki.dae@samsung.com, sw0312.kim@samsung.com,
	georgi.djakov@linaro.org, m.szyprowski@samsung.com
In-Reply-To: <20190723122016.30279-10-a.swigon@partner.samsung.com>

On 23.07.2019 15:21, Artur Świgoń wrote:

> +static int exynos_bus_icc_aggregate(struct icc_node *node, u32 avg_bw,
> +				    u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> +{
> +	*agg_peak = *agg_avg = peak_bw;
> +
> +	return 0;
> +}

The only current provider aggregates "avg" with "sum" and "peak" with 
"max", any particular reason to do something different? This function 
doesn't even really do aggregation, if there is a second request for "0" 
then the first request is lost.

I didn't find any docs but my interpretation of avg/peak is that "avg" 
is for constant traffic like a display or VPU pushing pixels and "peak" 
is for variable traffic like networking. I assume devices which make 
"peak" requests are aggregated with max because they're not expected to 
all max-out together.

In PATCH 11 you're making a bandwidth request based on resolution, that 
traffic is constant and guaranteed to happend while the display is on so 
it would make sense to request it as an "avg" and aggregate it with "sum".

--
Regards,
Leonard

^ permalink raw reply

* Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
From: Marek Vasut @ 2019-08-06 13:35 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, Mauro Carvalho Chehab, Rob Herring, devicetree
In-Reply-To: <20190701080457.acucsi6p3mlmbg75@paasikivi.fi.intel.com>

On 7/1/19 10:04 AM, Sakari Ailus wrote:
> Hi Marek,
> 
> On Mon, May 20, 2019 at 10:18:11PM +0200, Marek Vasut wrote:
>> Add bindings for the Intersil ISL7998x BT656-to-MIPI-CSI2 decoder.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> To: linux-media@vger.kernel.org
>> ---
>>  .../bindings/media/i2c/isl7998x.txt           | 37 +++++++++++++++++++
>>  1 file changed, 37 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/isl7998x.txt b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> new file mode 100644
>> index 000000000000..c21703983360
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> @@ -0,0 +1,37 @@
>> +Intersil ISL7998x BT656-to-MIPI-CSI2 decoder
>> +
>> +The Intersil ISL7998x is a BT656-to-MIPI-CSI decoder which, capable of
>> +receiving up to four analog stream and multiplexing them into up to four
>> +MIPI CSI2 virtual channels, using one MIPI clock lane and 1/2 data lanes.
>> +
>> +Required Properties:
>> +- compatible: value should be "isil,isl79987"
>> +- pd-gpios: a GPIO spec for the Power Down pin (active high)
>> +
>> +Option Properties:
>> +- isil,num-inputs: Number of connected inputs (1, 2 or 4)
> 
> The presence of ports describing connected Bt.656 inputs tells this.
> 
>> +
>> +For further reading on port node refer to
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> 
> Which endpoint properties are relevant for the endpoint(s) in the CSI-2 port?

clock-lanes, data-lanes and the remote-endpoint .

> How about the ports describing the Bt.656 interfaces? You should have
> those, too...

So who would be connected to these analog "input" endpoint ? What would
be their "remote-endpoint" ?

>> +
>> +Example:
>> +
>> +	i2c_master {
>> +		isl7998x_mipi@44 {
>> +			compatible = "isil,isl79987";
>> +			reg = <0x44>;
>> +			isil,num-inputs = <4>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_videoadc>;
>> +			pd-gpios = <&gpio3 27 GPIO_ACTIVE_HIGH>;
>> +			status = "okay";
>> +
>> +			port {
>> +				isl79987_to_mipi_csi2: endpoint {
>> +					remote-endpoint = <&mipi_csi2_in>;
>> +					clock-lanes = <0>;
>> +					data-lanes = <1 2>;
>> +				};
>> +			};
>> +		};
>> +	};
>>
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
From: Marek Vasut @ 2019-08-06 13:35 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Ian Arkver, linux-media, Sakari Ailus, Mauro Carvalho Chehab,
	Rob Herring, devicetree
In-Reply-To: <20190529134321.3naykdw4jx5xu3jl@uno.localdomain>

On 5/29/19 3:43 PM, Jacopo Mondi wrote:
> HI Marek,

Hi,

> On Wed, May 29, 2019 at 01:09:47PM +0200, Marek Vasut wrote:
>> On 5/29/19 1:04 PM, Ian Arkver wrote:
>>> Hi,
>>>
>>> On 29/05/2019 11:41, Marek Vasut wrote:
>>>> On 5/29/19 8:28 AM, Jacopo Mondi wrote:
>>>>
>>>> [...]
>>>>
>>>>>>>>> [1]
>>>>>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +- compatible: value should be "isil,isl79987"
>>>>>>>
>>>>>>> And here you might want to have 2 different compatibles for 79987 and
>>>>>>> 79988.
>>>>>>
>>>>>> The 79988 is not supported yet, do we want to have it in the binding
>>>>>> doc?
>>>>>>
>>>>>
>>>>> I got mislead by the isl7998x naming scheme you used...
>>>>>
>>>>> I would say that's up to you, the two chips seems very similar,
>>>>> and it might make sense to provide bindings that support both. At the
>>>>> same time, as long as the here defined bindings does not prevent
>>>>> future expansions to include the ISL79988, its support could be safely
>>>>> post-poned. In that case please s/isl7998x/isl79987/ in this document
>>>>> and do not mention BT565 in the description.
>>>>
>>>> Right
>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>> I see from the example you only support one output port? How do you
>>>>>>>>> model the input ones.
>>>>>>>>
>>>>>>>> I don't . Do we model analog inputs now somehow ?
>>>>>>>
>>>>>>> I really think so, please see:
>>>>>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>>>>>>
>>>>>>>
>>>>>>> And as an example of a board device tree using connectors to model
>>>>>>> analog input see how the cvbs input on Salvator-X is described:
>>>>>>>
>>>>>>>     cvbs-in {
>>>>>>>         compatible = "composite-video-connector";
>>>>>>>         label = "CVBS IN";
>>>>>>>
>>>>>>>         port {
>>>>>>>             cvbs_con: endpoint {
>>>>>>>                 remote-endpoint = <&adv7482_ain7>;
>>>>>>>             };
>>>>>>>         };
>>>>>>>     };
>>>>>>>
>>>>>>> I think you should provide 4 input ports, where to connect input from
>>>>>>> the analog connectors, and derive the number of enabled inputs from
>>>>>>> the number of endpoints connected to an active remote.
>>>>>>
>>>>>> Deriving the number of active physical inputs from some existing
>>>>>> binding
>>>>>> makes sense.
>>>>>>
>>>>>> However unlike the adv7482, the isl79987 does not support remapping the
>>>>>> physical inputs to ADCs in the chip. It does support some remapping of
>>>>>> physical inputs to MIPI CSI2 channels, but that's probably not very
>>>>>> useful.
>>>>>>
>>>>>
>>>>> I understand, but I will now use against you the argument you have
>>>>> correctly pointed out here below that DT should describe hardware, and
>>>>> the hardware has indeed 4 input ports..
>>>>
>>>> My question here is whether it makes sense to describe the ports even if
>>>> they cannot be muxed to different ADC. Does it ?
>>>
>>> Each input port can be either differential CVBS or single ended with a
>>> 2:1 input select mux. It would be nice to be able to describe this.
>>
>> Where do you see that ?
>>
>>> You cannot remap the inputs to different ADCs, but you can remap the
>>> ADCs to different VC IDs using the
>>> ISL7998x_REG_P5_LI_ENGINE_VC_ASSIGNMENT register. Describing each input
>>> would proivde somewhere to specify the vc-id.
>>
>> I think Jacopo mentioned above the input muxing and the MIPI CSI2 VC
>> muxing are two separate things. But I have to wonder, do we have a way
>> of muxing the VCs in the DT or via the media controller yet ?
> 
> I'm not sure I get what you mean with "input muxing", do you mean
> controlling which input is directed to which ADC ?

Yes

> I don't have the
> chip manual, but according to what you and Ian said this is not
> possible.

Correct. You can only remap ADC output(s) to different MIPI CSI2 VC(s).

> Selecting which input video stream is directed to which CSI-2 virtual
> channel in the output CSI-2 stream is not possible in mainline. There
> are patches in development that would allow you to do so, but their
> design is not fully finalized yet. They would, in any case, require
> you to have one sink pad for each input port

Right, and the iMX6 CSI2 driver which I use for development doesn't
support that yet either.

>, and while you could
> register as many pads as it is specified in your custom property,
> you would loose the notion of which input is connected to which port
> ie. when a single input (isil,num-inputs=1) is connected to an input
> port which is not the first one (anyway, quickly looking at 2/2 it
> seems to me you only register a single source pad for this device).
> 
> The DT bindings layout is an orthogonal problem though. My opinion is
> that as the chip has 4 available input connections it should have 4
> input ports in the bindings definition, and for each one you would
> register a sink pad.

Who would be the source for such an input "sink" pad though ? I looked
at
Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt,
as you suggested above, but this is for video _OUTPUT_ devices, not
input devices, so that's likely not what I want.

> Only the actually connected ones should be present
> in the DTS, so that the driver knows which input port is active and,
> once something that allows you to control VC will land in mainline,
> it will let you tell something like "I want the video stream
> received on the input port@2 sent in virtual channel x in the output
> CSI-2 stream", but again this is quite an orthogonal issue. Sure thing
> is that with the current design and implementation, which afaict does
> not provides any sink pad, this is frowned upon (but you can then say
> 'who cares, since it's not here yet' :)

Fine, so let's ignore this for now.

> Or what Ian said, if you can model a connector that provides 2 single
> ended inputs, each connected to an input port and muxed internally by
> the chip, you could only do that if you have numbered input ports I
> guess. Nobody requires you support any of these, but at least bindings
> should be defined in a future proof way, to avoid later changes to the
> ABI.

Well, if endpoint 0 is the MIPI CSI2 output, then you can always add
more endpoints (1..4) for the analog inputs . In this way , the bindings
are already future-proof.

Or shall the input ports come first (as endpoints 0..3) and be followed
by the MIPI CSI2 output (4) ? If so, that would make counting the
(varying number of) inputs a bit cumbersome in the driver.

> Anyway, you had my opinion, multiple times already, and as I'm not in
> any position to judge if something is acceptable for merge or not,
> I'll now get quiet hoping that this prolonged email exchange drags in
> opinion from more experienced people to tell you which direction you
> should take and if what you have here is fine already or not :)
> 
> Cheers
>    j
> 
>>
>> --
>> Best regards,
>> Marek Vasut

-- 
Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
From: Marek Vasut @ 2019-08-06 13:35 UTC (permalink / raw)
  To: Ian Arkver, Jacopo Mondi
  Cc: linux-media, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
	devicetree
In-Reply-To: <040ee008-2ae9-40d3-fa2f-729e7da4331a@gmail.com>

On 5/29/19 1:15 PM, Ian Arkver wrote:
> Hi Marek,
> 
> On 29/05/2019 12:09, Marek Vasut wrote:
>> On 5/29/19 1:04 PM, Ian Arkver wrote:
>>> Hi,
>>>
>>> On 29/05/2019 11:41, Marek Vasut wrote:
>>>> On 5/29/19 8:28 AM, Jacopo Mondi wrote:
>>>>
>>>> [...]
>>>>
>>>>>>>>> [1]
>>>>>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +- compatible: value should be "isil,isl79987"
>>>>>>>
>>>>>>> And here you might want to have 2 different compatibles for 79987
>>>>>>> and
>>>>>>> 79988.
>>>>>>
>>>>>> The 79988 is not supported yet, do we want to have it in the binding
>>>>>> doc?
>>>>>>
>>>>>
>>>>> I got mislead by the isl7998x naming scheme you used...
>>>>>
>>>>> I would say that's up to you, the two chips seems very similar,
>>>>> and it might make sense to provide bindings that support both. At the
>>>>> same time, as long as the here defined bindings does not prevent
>>>>> future expansions to include the ISL79988, its support could be safely
>>>>> post-poned. In that case please s/isl7998x/isl79987/ in this document
>>>>> and do not mention BT565 in the description.
>>>>
>>>> Right
>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>> I see from the example you only support one output port? How do
>>>>>>>>> you
>>>>>>>>> model the input ones.
>>>>>>>>
>>>>>>>> I don't . Do we model analog inputs now somehow ?
>>>>>>>
>>>>>>> I really think so, please see:
>>>>>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> And as an example of a board device tree using connectors to model
>>>>>>> analog input see how the cvbs input on Salvator-X is described:
>>>>>>>
>>>>>>>      cvbs-in {
>>>>>>>          compatible = "composite-video-connector";
>>>>>>>          label = "CVBS IN";
>>>>>>>
>>>>>>>          port {
>>>>>>>              cvbs_con: endpoint {
>>>>>>>                  remote-endpoint = <&adv7482_ain7>;
>>>>>>>              };
>>>>>>>          };
>>>>>>>      };
>>>>>>>
>>>>>>> I think you should provide 4 input ports, where to connect input
>>>>>>> from
>>>>>>> the analog connectors, and derive the number of enabled inputs from
>>>>>>> the number of endpoints connected to an active remote.
>>>>>>
>>>>>> Deriving the number of active physical inputs from some existing
>>>>>> binding
>>>>>> makes sense.
>>>>>>
>>>>>> However unlike the adv7482, the isl79987 does not support
>>>>>> remapping the
>>>>>> physical inputs to ADCs in the chip. It does support some
>>>>>> remapping of
>>>>>> physical inputs to MIPI CSI2 channels, but that's probably not very
>>>>>> useful.
>>>>>>
>>>>>
>>>>> I understand, but I will now use against you the argument you have
>>>>> correctly pointed out here below that DT should describe hardware, and
>>>>> the hardware has indeed 4 input ports..
>>>>
>>>> My question here is whether it makes sense to describe the ports
>>>> even if
>>>> they cannot be muxed to different ADC. Does it ?
>>>
>>> Each input port can be either differential CVBS or single ended with a
>>> 2:1 input select mux. It would be nice to be able to describe this.
>>
>> Where do you see that ?
> 
> Bits 0 and 1 of each channel page's Differential Clamping Control 4
> (0x39, ISL7998x_REG_Px_DEC_DIFF_CLMP_CTL_4).
> 
> I don't think you change it from the default (single ended on the first
> input).

I don't, since I have no way to test the differential mode of operation.

[...]

^ permalink raw reply

* Re: [RESEND V2 1/2] dt-bindings: Document the DesignWare IP reset bindings
From: Philipp Zabel @ 2019-08-06 13:33 UTC (permalink / raw)
  To: Luis Oliveira, robh+dt, mark.rutland, linux-kernel, devicetree
  Cc: Joao.Pinto, Gustavo Pimentel
In-Reply-To: <1563895048-30038-2-git-send-email-luis.oliveira@synopsys.com>

Hi Luis,

On Tue, 2019-07-23 at 17:17 +0200, Luis Oliveira wrote:
> This adds documentation of device tree bindings for the
> DesignWare IP reset controller.
> 
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Signed-off-by: Luis Oliveira <luis.oliveira@synopsys.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Thank you, both applied to reset/next.

regards
Philipp

^ permalink raw reply

* Re: [PATCH v8 00/11] add USB GPIO based connection detection driver
From: Linus Walleij @ 2019-08-06 13:26 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Rob Herring, Greg Kroah-Hartman, Biju Das, Mark Rutland,
	Matthias Brugger, Adam Thomson, Li Jun, Badhri Jagan Sridharan,
	Heikki Krogerus, Hans de Goede, Andy Shevchenko, Min Guo,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel@vger.kernel.org, linux-usb, Linux ARM, ARM/Mediatek 
In-Reply-To: <1565077710.23705.21.camel@mhfsdcap03>

On Tue, Aug 6, 2019 at 9:48 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> On Mon, 2019-08-05 at 12:06 +0200, Linus Walleij wrote:
> > On Wed, Jul 24, 2019 at 10:51 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:

> > But for just "hey I'm plugged in" we can surely keep this
> > ID on GPIO detection in the USB subsystem.
>
> Sorry, you mean provide a common API? could you please describe more
> clearer about your suggestion?

Sorry I am not suggesting anything, this code is fine.

But:

> > I just get a bit insecure about how we should ideally
> > handle these "funny-PHY's".

I am more thinking about which subsystem these things really
belong in. But let's keep it like this for the simple GPIO case.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v1 2/2] net: npcm: add NPCM7xx EMC 10/100 Ethernet driver
From: Avi Fishman @ 2019-08-06 13:19 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Patrick Venture, Nancy Yuen, Benjamin Fair, David Miller,
	Rob Herring, Mark Rutland, Greg Kroah-Hartman, Tomer Maimon,
	Tali Perry, OpenBMC Maillist, Network Development, devicetree,
	linux-kernel, Thomas Gleixner
In-Reply-To: <CA+FuTSd89gJBX-zaZTzgNxpqtR_MvVfMf=6hdRe5+1MPRszw8g@mail.gmail.com>

Thanks for the input Willem,

Before I will submit a new version please help me with some questions:

On Thu, Aug 1, 2019 at 8:26 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Thu, Aug 1, 2019 at 3:28 AM Avi Fishman <avifishman70@gmail.com> wrote:
> >
> > EMC Ethernet Media Access Controller supports 10/100 Mbps and
> > RMII.
> > This driver has been working on Nuvoton BMC NPCM7xx.
> >
> > Signed-off-by: Avi Fishman <avifishman70@gmail.com>
>
>
>
> > +/* global setting for driver */
> > +#define RX_QUEUE_LEN   128
> > +#define TX_QUEUE_LEN   64
> > +#define MAX_RBUFF_SZ   0x600
> > +#define MAX_TBUFF_SZ   0x600
> > +#define TX_TIMEOUT     50
> > +#define DELAY          1000
> > +#define CAM0           0x0
> > +#define RX_POLL_SIZE    16
> > +
> > +#ifdef CONFIG_VLAN_8021Q
> > +#define IS_VLAN 1
> > +#else
> > +#define IS_VLAN 0
> > +#endif
> > +
> > +#define MAX_PACKET_SIZE           (1514 + (IS_VLAN * 4))
>
> 1514 -> ETH_FRAME_LEN
>
> 4 -> VLAN_HLEN

OK

>
> Does this device support stacked VLAN?
I am not familiar with stacked VLAN.
Our HW for sure there is no support. can the SW stack handle it for me?
Does it mean that  the packets can be larger?

>
> Is this really the device maximum?

The device can support upto 64KB, but of course I will not allocate
for each RX data such a big buffer.
Can I know what is the maximum value the network stack may request? I
saw many driver allocating 1536 for each packet.

>
> > +#define MAX_PACKET_SIZE_W_CRC     (MAX_PACKET_SIZE + 4) /* 1518 */
>
> 4 -> ETH_FCS_LEN

OK

>
> > +#if defined CONFIG_NPCM7XX_EMC_ETH_DEBUG || defined CONFIG_DEBUG_FS
> > +#define REG_PRINT(reg_name) {t = scnprintf(next, size, "%-10s = %08X\n", \
> > +       #reg_name, readl(ether->reg + (reg_name))); size -= t;  next += t; }
> > +#define DUMP_PRINT(f, x...) {t = scnprintf(next, size, f, ## x); size -= t; \
> > +       next += t; }
> > +
> > +static int npcm7xx_info_dump(char *buf, int count, struct net_device *netdev)
> > +{
> > +       struct npcm7xx_ether *ether = netdev_priv(netdev);
> > +       struct npcm7xx_txbd *txbd;
> > +       struct npcm7xx_rxbd *rxbd;
> > +       unsigned long flags;
> > +       unsigned int i, cur, txd_offset, rxd_offset;
> > +       char *next = buf;
> > +       unsigned int size = count;
> > +       int t;
> > +       int is_locked = spin_is_locked(&ether->lock);
> > +
> > +       if (!is_locked)
> > +               spin_lock_irqsave(&ether->lock, flags);
> > +
> > +       /* ------basic driver information ---- */
> > +       DUMP_PRINT("NPCM7XX EMC %s driver version: %s\n", netdev->name,
> > +                  DRV_MODULE_VERSION);
> > +
> > +       REG_PRINT(REG_CAMCMR);
> > +       REG_PRINT(REG_CAMEN);
> > +       REG_PRINT(REG_CAMM_BASE);
> > +       REG_PRINT(REG_CAML_BASE);
> > +       REG_PRINT(REG_TXDLSA);
> > +       REG_PRINT(REG_RXDLSA);
> > +       REG_PRINT(REG_MCMDR);
> > +       REG_PRINT(REG_MIID);
> > +       REG_PRINT(REG_MIIDA);
> > +       REG_PRINT(REG_FFTCR);
> > +       REG_PRINT(REG_TSDR);
> > +       REG_PRINT(REG_RSDR);
> > +       REG_PRINT(REG_DMARFC);
> > +       REG_PRINT(REG_MIEN);
> > +       REG_PRINT(REG_MISTA);
> > +       REG_PRINT(REG_MGSTA);
> > +       REG_PRINT(REG_MPCNT);
> > +       writel(0x7FFF, (ether->reg + REG_MPCNT));
> > +       REG_PRINT(REG_MRPC);
> > +       REG_PRINT(REG_MRPCC);
> > +       REG_PRINT(REG_MREPC);
> > +       REG_PRINT(REG_DMARFS);
> > +       REG_PRINT(REG_CTXDSA);
> > +       REG_PRINT(REG_CTXBSA);
> > +       REG_PRINT(REG_CRXDSA);
> > +       REG_PRINT(REG_CRXBSA);
> > +       REG_PRINT(REG_RXFSM);
> > +       REG_PRINT(REG_TXFSM);
> > +       REG_PRINT(REG_FSM0);
> > +       REG_PRINT(REG_FSM1);
> > +       REG_PRINT(REG_DCR);
> > +       REG_PRINT(REG_DMMIR);
> > +       REG_PRINT(REG_BISTR);
> > +       DUMP_PRINT("\n");
> > +
> > +       DUMP_PRINT("netif_queue %s\n\n", netif_queue_stopped(netdev) ?
> > +                                       "Stopped" : "Running");
> > +       if (ether->rdesc)
> > +               DUMP_PRINT("napi is %s\n\n", test_bit(NAPI_STATE_SCHED,
> > +                                                     &ether->napi.state) ?
> > +                                                       "scheduled" :
> > +                                                       "not scheduled");
> > +
> > +       txd_offset = (readl((ether->reg + REG_CTXDSA)) -
> > +                     readl((ether->reg + REG_TXDLSA))) /
> > +               sizeof(struct npcm7xx_txbd);
> > +       DUMP_PRINT("TXD offset    %6d\n", txd_offset);
> > +       DUMP_PRINT("cur_tx        %6d\n", ether->cur_tx);
> > +       DUMP_PRINT("finish_tx     %6d\n", ether->finish_tx);
> > +       DUMP_PRINT("pending_tx    %6d\n", ether->pending_tx);
> > +       /* debug counters */
> > +       DUMP_PRINT("tx_tdu        %6d\n", ether->tx_tdu);
> > +       ether->tx_tdu = 0;
> > +       DUMP_PRINT("tx_tdu_i      %6d\n", ether->tx_tdu_i);
> > +       ether->tx_tdu_i = 0;
> > +       DUMP_PRINT("tx_cp_i       %6d\n", ether->tx_cp_i);
> > +        ether->tx_cp_i = 0;
> > +       DUMP_PRINT("tx_int_count  %6d\n", ether->tx_int_count);
> > +       ether->tx_int_count = 0;
> > +       DUMP_PRINT("count_xmit tx %6d\n", ether->count_xmit);
> > +       ether->count_xmit = 0;
> > +       DUMP_PRINT("count_finish  %6d\n", ether->count_finish);
> > +       ether->count_finish = 0;
> > +       DUMP_PRINT("\n");
> > +
> > +       rxd_offset = (readl((ether->reg + REG_CRXDSA)) -
> > +                     readl((ether->reg + REG_RXDLSA)))
> > +               / sizeof(struct npcm7xx_txbd);
> > +       DUMP_PRINT("RXD offset    %6d\n", rxd_offset);
> > +       DUMP_PRINT("cur_rx        %6d\n", ether->cur_rx);
> > +       DUMP_PRINT("rx_err        %6d\n", ether->rx_err);
> > +       ether->rx_err = 0;
> > +       DUMP_PRINT("rx_berr       %6d\n", ether->rx_berr);
> > +       ether->rx_berr = 0;
> > +       DUMP_PRINT("rx_stuck      %6d\n", ether->rx_stuck);
> > +       ether->rx_stuck = 0;
> > +       DUMP_PRINT("rdu           %6d\n", ether->rdu);
> > +       ether->rdu = 0;
> > +       DUMP_PRINT("rxov rx       %6d\n", ether->rxov);
> > +       ether->rxov = 0;
> > +       /* debug counters */
> > +       DUMP_PRINT("rx_int_count  %6d\n", ether->rx_int_count);
> > +       ether->rx_int_count = 0;
> > +       DUMP_PRINT("rx_err_count  %6d\n", ether->rx_err_count);
> > +       ether->rx_err_count = 0;
>
> Basic counters like tx_packets and rx_errors are probably better
> exported regardless of debug level as net_device_stats. And then don't
> need to be copied in debug output.

They are also exported there, see below ether->stats.tx_packets++; and
ether->stats.rx_errors++;
those are different counters for debug since we had HW issues that we
needed to workaround and might need them for future use.
Currently the driver is stable on millions of parts in the field.

>
> Less standard counters like tx interrupt count are probably better
> candidates for ethtool -S.

I don't have support for ethtool.
is it a must? it is quite some work and this driver is already in the
field for quite some years.

>
> > +#ifdef CONFIG_NPCM7XX_EMC_ETH_DEBUG
> > +static void npcm7xx_info_print(struct net_device *netdev)
> > +{
> > +       char *emc_dump_buf;
> > +       int count;
> > +       struct npcm7xx_ether *ether;
> > +       struct platform_device *pdev;
> > +       char c;
> > +       char *tmp_buf;
> > +       const size_t print_size = 5 * PAGE_SIZE;

I will change print_size to 0x5000 since PAGE_SIZE is not a fixed
value on all arch.

> > +
> > +       ether = netdev_priv(netdev);
> > +       pdev = ether->pdev;
> > +
> > +       emc_dump_buf = kmalloc(print_size, GFP_KERNEL);
> > +       if (!emc_dump_buf)
> > +               return;
> > +
> > +       tmp_buf = emc_dump_buf;
> > +       count = ether->stats.rx_errors++;(emc_dump_buf, print_size, netdev);
> > +       while (count > 512) {
> > +               c = tmp_buf[512];
> > +               tmp_buf[512] = 0;
> > +               dev_info(&pdev->dev, "%s", tmp_buf);
> > +               tmp_buf += 512;
> > +               tmp_buf[0] = c;
> > +               count -= 512;
>
> Missing closing parenthesis.

WOW! good catch, before submitting I made few change due to
checkpatch.pl and didn't compile with CONFIG_NPCM7XX_EMC_ETH_DEBUG

>
> Also, why this buffering to printk?

I prepare the buffer in advance for real-time reasons because of the
lock in order to get a correct snapshot of the registers.
The buffer is big and I saw that printk has limited length so I split it.

>
> > +static void npcm7xx_write_cam(struct net_device *netdev,
> > +                             unsigned int x, unsigned char *pval)
> > +{
> > +       struct npcm7xx_ether *ether = netdev_priv(netdev);
> > +       __le32 msw, lsw;
> > +
> > +       msw = (pval[0] << 24) | (pval[1] << 16) | (pval[2] << 8) | pval[3];
> > +
> > +       lsw = (pval[4] << 24) | (pval[5] << 16);
>
> Does __le32 plus this explicit shifting define host endianness? Better
> to keep independent?

OK

>
> > +
> > +       writel(lsw, (ether->reg + REG_CAML_BASE) + x * CAM_ENTRY_SIZE);
> > +       writel(msw, (ether->reg + REG_CAMM_BASE) + x * CAM_ENTRY_SIZE);
> > +       dev_dbg(&ether->pdev->dev,
> > +               "REG_CAML_BASE = 0x%08X REG_CAMM_BASE = 0x%08X", lsw, msw);
> > +}
> > +
> > +static struct sk_buff *get_new_skb(struct net_device *netdev, u32 i)
> > +{
> > +       __le32 buffer;
> > +       struct npcm7xx_ether *ether = netdev_priv(netdev);
> > +       struct sk_buff *skb = netdev_alloc_skb(netdev,
> > +               roundup(MAX_PACKET_SIZE_W_CRC, 4));
> > +
> > +       if (unlikely(!skb)) {
> > +               if (net_ratelimit())
> > +                       netdev_warn(netdev, "failed to allocate rx skb\n");
>
> can use net_warn_ratelimited (here and elsewhere)

should I replace every netdev_warn/err/info with net_warn/err/inf_ratelimited?
I saw in drivers that are using net_warn_ratelimited, that many time
uses also netdev_warn/err/info

>
> > +               buffer = ether->rx_scratch_dma;
> > +       } else {
> > +               /* Do not unmark the following skb_reserve() Receive Buffer
> > +                * Starting Address must be aligned to 4 bytes and the following
> > +                * line if unmarked will make it align to 2 and this likely will
> > +                * hult the RX and crash the linux
>
> halt?

will fix typo.

>
> > +                * skb_reserve(skb, NET_IP_ALIGN);
> > +                */
> > +               skb->dev = netdev;
> > +               buffer = dma_map_single(&netdev->dev,
> > +                                       skb->data,
> > +                                       roundup(MAX_PACKET_SIZE_W_CRC, 4),
> > +                                       DMA_FROM_DEVICE);
> > +               if (unlikely(dma_mapping_error(&netdev->dev, buffer))) {
> > +                       if (net_ratelimit())
> > +                               netdev_err(netdev, "failed to map rx page\n");
> > +                       dev_kfree_skb_any(skb);
> > +                       buffer = ether->rx_scratch_dma;
> > +                       skb = NULL;
> > +               }
> > +       }
> > +       ether->rx_skb[i] = skb;
> > +       ether->rdesc[i].buffer = buffer;
> > +
> > +       return skb;
> > +}
> > +
>
> > +static int npcm7xx_ether_close(struct net_device *netdev)
> > +{
> > +       struct npcm7xx_ether *ether = netdev_priv(netdev);
> > +
> > +       npcm7xx_return_default_idle(netdev);
> > +
> > +       if (ether->phy_dev)
> > +               phy_stop(ether->phy_dev);
> > +       else if (ether->use_ncsi)
> > +               ncsi_stop_dev(ether->ncsidev);
> > +
> > +       msleep(20);
> > +
> > +       free_irq(ether->txirq, netdev);
> > +       free_irq(ether->rxirq, netdev);
> > +
> > +       netif_stop_queue(netdev);
> > +       napi_disable(&ether->napi);
>
> Cleanup state in reverse of allocation.

OK

>
> > +static int npcm7xx_ether_start_xmit(struct sk_buff *skb, struct net_device *netdev)
> > +{
> > +       struct npcm7xx_ether *ether = netdev_priv(netdev);
> > +       struct npcm7xx_txbd *txbd;
> > +       unsigned long flags;
> > +
> > +       ether->count_xmit++;
> > +
> > +       /* Insert new buffer */
> > +       txbd = (ether->tdesc + ether->cur_tx);
> > +       txbd->buffer = dma_map_single(&netdev->dev, skb->data, skb->len,
> > +                                     DMA_TO_DEVICE);
> > +       ether->tx_skb[ether->cur_tx]  = skb;
> > +       if (skb->len > MAX_PACKET_SIZE)
> > +               dev_err(&ether->pdev->dev,
> > +                       "skb->len (= %d) > MAX_PACKET_SIZE (= %d)\n",
> > +                       skb->len, MAX_PACKET_SIZE);
>
> > +       txbd->sl = skb->len > MAX_PACKET_SIZE ? MAX_PACKET_SIZE : skb->len;
>
> Check for errors before mapping to device, and drop packet? Probably
> don't want to output truncated packets.
>
> Also rate limit such messages.

OK

>
> > +       dma_wmb();
> > +
> > +       txbd->mode = TX_OWN_DMA | PADDINGMODE | CRCMODE;
> > +
> > +       /* make sure that data is in memory before we trigger TX */
> > +       wmb();
> > +
> > +       /* trigger TX */
> > +       writel(ENSTART, (ether->reg + REG_TSDR));
> > +
> > +       if (++ether->cur_tx >= TX_QUEUE_LEN)
> > +               ether->cur_tx = 0;
> > +
> > +       spin_lock_irqsave(&ether->lock, flags);
> > +       ether->pending_tx++;
> > +
> > +       /* sometimes we miss the tx interrupt due to HW issue, so NAPI will not
> > +        * clean the pending tx, so we clean it also here
> > +        */
> > +       npcm7xx_clean_tx(netdev, true);
> > +
> > +       if (ether->pending_tx >= TX_QUEUE_LEN - 1) {
> > +               __le32 reg_mien;
> > +               unsigned int index_to_wake = ether->cur_tx +
> > +                       ((TX_QUEUE_LEN * 3) / 4);
> > +
> > +               if (index_to_wake >= TX_QUEUE_LEN)
> > +                       index_to_wake -= TX_QUEUE_LEN;
> > +
> > +               txbd = (ether->tdesc + index_to_wake);
> > +               txbd->mode = TX_OWN_DMA | PADDINGMODE | CRCMODE | MACTXINTEN;
> > +
> > +               /* make sure that data is in memory before we trigger TX */
> > +               wmb();
> > +
> > +               /* Clear TDU interrupt */
> > +               writel(MISTA_TDU, (ether->reg + REG_MISTA));
> > +
> > +               /* due to HW issue somtimes, we miss the TX interrupt we just
>
> somtimes -> sometimes

OK

>
> > +                * set (MACTXINTEN), so we also set TDU for Transmit
> > +                * Descriptor Unavailable interrupt
> > +                */
> > +               reg_mien = readl((ether->reg + REG_MIEN));
> > +               if (reg_mien != 0)
> > +                       /* Enable TDU interrupt */
> > +                       writel(reg_mien | ENTDU, (ether->reg + REG_MIEN));
> > +
> > +               ether->tx_tdu++;
> > +               netif_stop_queue(netdev);
> > +       }
> > +
> > +       spin_unlock_irqrestore(&ether->lock, flags);
> > +
> > +       return 0;
> > +}
> > +
> > +static irqreturn_t npcm7xx_tx_interrupt(int irq, void *dev_id)
> > +{
> > +       struct npcm7xx_ether *ether;
> > +       struct platform_device *pdev;
> > +       struct net_device *netdev;
> > +       __le32 status;
> > +       unsigned long flags;
> > +
> > +       netdev = dev_id;
> > +       ether = netdev_priv(netdev);
> > +       pdev = ether->pdev;
> > +
> > +       npcm7xx_get_and_clear_int(netdev, &status, 0xFFFF0000);
> > +
> > +       ether->tx_int_count++;
> > +
> > +       if (status & MISTA_EXDEF)
> > +               dev_err(&pdev->dev, "emc defer exceed interrupt status=0x%08X\n"
> > +                       , status);
> > +       else if (status & MISTA_TXBERR) {
> > +               dev_err(&pdev->dev, "emc bus error interrupt status=0x%08X\n",
> > +                       status);
> > +#ifdef CONFIG_NPCM7XX_EMC_ETH_DEBUG
> > +               npcm7xx_info_print(netdev);
> > +#endif
> > +               spin_lock_irqsave(&ether->lock, flags);
>
> irqsave in hard interrupt context?

I need to protect my REG_MIEN register that is changed in other places.
I think that spin_lock_irqsave() is relevant when working in SMP,
since other CPU may still be running.
Isn't it?

>
> > +               writel(0, (ether->reg + REG_MIEN)); /* disable any interrupt */
> > +               spin_unlock_irqrestore(&ether->lock, flags);
> > +               ether->need_reset = 1;
> > +       } else if (status & ~(MISTA_TXINTR | MISTA_TXCP | MISTA_TDU))
> > +               dev_err(&pdev->dev, "emc other error interrupt status=0x%08X\n",
> > +                       status);
> > +
> > +    /* if we got MISTA_TXCP | MISTA_TDU remove those interrupt and call napi */
>
> The goal of napi is to keep interrupts disabled until napi completes.

We have a HW issue that because of it I still enabled TX complete interrupt,
I will see if I can disable all interrupts and only leave the error interrupts

>
> > +       if (status & (MISTA_TXCP | MISTA_TDU) &
> > +           readl((ether->reg + REG_MIEN))) {
> > +               __le32 reg_mien;
> > +
> > +               spin_lock_irqsave(&ether->lock, flags);
> > +               reg_mien = readl((ether->reg + REG_MIEN));
> > +               if (reg_mien & ENTDU)
> > +                       /* Disable TDU interrupt */
> > +                       writel(reg_mien & (~ENTDU), (ether->reg + REG_MIEN));
> > +
> > +               spin_unlock_irqrestore(&ether->lock, flags);
> > +
> > +               if (status & MISTA_TXCP)
> > +                       ether->tx_cp_i++;
> > +               if (status & MISTA_TDU)
> > +                       ether->tx_tdu_i++;
> > +       } else {
> > +               dev_dbg(&pdev->dev, "status=0x%08X\n", status);
> > +       }
> > +
> > +       napi_schedule(&ether->napi);
> > +
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t npcm7xx_rx_interrupt(int irq, void *dev_id)
> > +{
> > +       struct net_device *netdev = (struct net_device *)dev_id;
> > +       struct npcm7xx_ether *ether = netdev_priv(netdev);
> > +       struct platform_device *pdev = ether->pdev;
> > +       __le32 status;
> > +       unsigned long flags;
> > +       unsigned int any_err = 0;
> > +       __le32 rxfsm;
> > +
> > +       npcm7xx_get_and_clear_int(netdev, &status, 0xFFFF);
>
> Same here

in non error case I do leave only the error interrupts and schedule napi.

>
> > +static int npcm7xx_poll(struct napi_struct *napi, int budget)
> > +{
> > +       struct npcm7xx_ether *ether =
> > +               container_of(napi, struct npcm7xx_ether, napi);
> > +       struct npcm7xx_rxbd *rxbd;
> > +       struct net_device *netdev = ether->netdev;
> > +       struct platform_device *pdev = ether->pdev;
> > +       struct sk_buff *s;
> > +       unsigned int length;
> > +       __le32 status;
> > +       unsigned long flags;
> > +       int rx_cnt = 0;
> > +       int complete = 0;
> > +       unsigned int rx_offset = (readl((ether->reg + REG_CRXDSA)) -
> > +                                 ether->start_rx_ptr) /
> > +                               sizeof(struct npcm7xx_txbd);
> > +       unsigned int local_count = (rx_offset >= ether->cur_rx) ?
> > +               rx_offset - ether->cur_rx : rx_offset +
> > +               RX_QUEUE_LEN - ether->cur_rx;
> > +
> > +       if (local_count > ether->max_waiting_rx)
> > +               ether->max_waiting_rx = local_count;
> > +
> > +       if (local_count > (4 * RX_POLL_SIZE))
> > +               /* we are porbably in a storm of short packets and we don't
>
> porbably - probably

OK

>
> > +                * want to get into RDU since short packets in RDU cause
> > +                * many RXOV which may cause EMC halt, so we filter out all
> > +                * coming packets
> > +                */
> > +               writel(0, (ether->reg + REG_CAMCMR));
> > +
> > +       if (local_count <= budget)
> > +               /* we can restore accepting of packets */
> > +               writel(ether->camcmr, (ether->reg + REG_CAMCMR));
> > +
> > +       spin_lock_irqsave(&ether->lock, flags);
> > +       npcm7xx_clean_tx(netdev, false);
> > +       spin_unlock_irqrestore(&ether->lock, flags);
> > +
> > +       rxbd = (ether->rdesc + ether->cur_rx);
> > +
> > +       while (rx_cnt < budget) {
> > +               status = rxbd->sl;
> > +               if ((status & RX_OWN_DMA) == RX_OWN_DMA) {
> > +                       complete = 1;
> > +                       break;
> > +               }
> > +               /* for debug puposes we save the previous value */
>
> puposes -> purposes

OK

>
> > +               rxbd->reserved = status;
> > +               s = ether->rx_skb[ether->cur_rx];
> > +               length = status & 0xFFFF;
> > +
> > +               /* If VLAN is not supporte RXDS_PTLE (packet too long) is also
>
> supporte -> supported (stopping pointing out typos after this).

OK will review rest


>
> > +static const struct net_device_ops npcm7xx_ether_netdev_ops = {
> > +       .ndo_open               = npcm7xx_ether_open,
> > +       .ndo_stop               = npcm7xx_ether_close,
> > +       .ndo_start_xmit         = npcm7xx_ether_start_xmit,
> > +       .ndo_get_stats          = npcm7xx_ether_stats,
> > +       .ndo_set_rx_mode        = npcm7xx_ether_set_rx_mode,
> > +       .ndo_set_mac_address    = npcm7xx_set_mac_address,
> > +       .ndo_do_ioctl           = npcm7xx_ether_ioctl,
> > +       .ndo_validate_addr      = eth_validate_addr,
> > +       .ndo_change_mtu         = eth_change_mtu,
>
> This is marked as deprecated. Also in light of the hardcoded
> MAX_PACKET_SIZE, probably want to set dev->max_mtu.

can I just not set .ndo_change_mtu? or I must add my own implementation?
setting of dev->max_mtu, can be done in probe, yes?
BTW, I see that currently the mtu is 1500 but I do get transactions
with len of 1514 (I didn't compile with VLAN)

>
> > +static int npcm7xx_ether_probe(struct platform_device *pdev)
> > +{
> > +       struct npcm7xx_ether *ether;
> > +       struct net_device *netdev;
> > +       int error;
> > +
> > +       struct clk *emc_clk = NULL;
> > +       struct device_node *np = pdev->dev.of_node;
> > +
> > +       pdev->id = of_alias_get_id(np, "ethernet");
> > +       if (pdev->id < 0)
> > +               pdev->id = 0;
> > +
> > +       emc_clk = devm_clk_get(&pdev->dev, NULL);
> > +
> > +       if (IS_ERR(emc_clk))
> > +               return PTR_ERR(emc_clk);
> > +
> > +       /* Enable Clock */
> > +       clk_prepare_enable(emc_clk);
> > +
> > +       error = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > +       if (error)
> > +               return -ENODEV;
> > +
> > +       netdev = alloc_etherdev(sizeof(struct npcm7xx_ether));
> > +       if (!netdev)
> > +               return -ENOMEM;
> > +
> > +       ether = netdev_priv(netdev);
> > +
> > +       ether->reset = devm_reset_control_get(&pdev->dev, NULL);
> > +       if (IS_ERR(ether->reset))
> > +               return PTR_ERR(ether->reset);
>
> Memory leak on error path
>
> Also missing netif_napi_del in npcm7xx_ether_remove?

added


--
Regards,
Avi

^ permalink raw reply

* [PATCH v2 3/3] MAINTAINERS: Add entry for IMX290 CMOS image sensor driver
From: Manivannan Sadhasivam @ 2019-08-06 13:09 UTC (permalink / raw)
  To: mchehab, robh+dt
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, Manivannan Sadhasivam
In-Reply-To: <20190806130938.19916-1-manivannan.sadhasivam@linaro.org>

Add MAINTAINERS entry for Sony IMX290 CMOS image sensor driver.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d0ed735994a5..27e4c1f57b61 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14669,6 +14669,14 @@ S:	Maintained
 F:	drivers/media/i2c/imx274.c
 F:	Documentation/devicetree/bindings/media/i2c/imx274.txt
 
+SONY IMX290 SENSOR DRIVER
+M:	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+L:	linux-media@vger.kernel.org
+T:	git git://linuxtv.org/media_tree.git
+S:	Maintained
+F:	drivers/media/i2c/imx290.c
+F:	Documentation/devicetree/bindings/media/i2c/imx290.txt
+
 SONY IMX319 SENSOR DRIVER
 M:	Bingbu Cao <bingbu.cao@intel.com>
 L:	linux-media@vger.kernel.org
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 2/3] media: i2c: Add IMX290 CMOS image sensor driver
From: Manivannan Sadhasivam @ 2019-08-06 13:09 UTC (permalink / raw)
  To: mchehab, robh+dt
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, Manivannan Sadhasivam
In-Reply-To: <20190806130938.19916-1-manivannan.sadhasivam@linaro.org>

Add driver for Sony IMX290 CMOS image sensor driver. The driver only
supports I2C interface for programming and MIPI CSI-2 for sensor output.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/media/i2c/Kconfig  |  11 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/imx290.c | 845 +++++++++++++++++++++++++++++++++++++
 3 files changed, 857 insertions(+)
 create mode 100644 drivers/media/i2c/imx290.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb8db944aa41..256edd289abe 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -594,6 +594,17 @@ config VIDEO_IMX274
 	  This is a V4L2 sensor driver for the Sony IMX274
 	  CMOS image sensor.
 
+config VIDEO_IMX290
+	tristate "Sony IMX290 sensor support"
+	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+	depends on MEDIA_CAMERA_SUPPORT
+	help
+	  This is a Video4Linux2 sensor driver for the Sony
+	  IMX290 camera sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called imx290.
+
 config VIDEO_IMX319
 	tristate "Sony IMX319 sensor support"
 	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d8ad9dad495d..490e59070407 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -111,6 +111,7 @@ obj-$(CONFIG_VIDEO_TC358743)	+= tc358743.o
 obj-$(CONFIG_VIDEO_IMX214)	+= imx214.o
 obj-$(CONFIG_VIDEO_IMX258)	+= imx258.o
 obj-$(CONFIG_VIDEO_IMX274)	+= imx274.o
+obj-$(CONFIG_VIDEO_IMX290)	+= imx290.o
 obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
 obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
 obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
new file mode 100644
index 000000000000..8c50fbd51ca5
--- /dev/null
+++ b/drivers/media/i2c/imx290.c
@@ -0,0 +1,845 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sony IMX290 CMOS Image Sensor Driver
+ *
+ * Copyright (C) 2019 FRAMOS GmbH.
+ *
+ * Copyright (C) 2019 Linaro Ltd.
+ * Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <media/media-entity.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+#define IMX290_STANDBY 0x3000
+#define IMX290_REGHOLD 0x3001
+#define IMX290_XMSTA 0x3002
+#define IMX290_GAIN 0x3014
+
+static const char * const imx290_supply_name[] = {
+	"vdda",
+	"vddd",
+	"vdddo",
+};
+
+#define IMX290_NUM_SUPPLIES ARRAY_SIZE(imx290_supply_name)
+
+struct imx290_regval {
+	u16 reg;
+	u8 val;
+};
+
+struct imx290_mode {
+	u32 width;
+	u32 height;
+	u32 pixel_rate;
+	u32 link_freq_index;
+
+	const struct imx290_regval *data;
+	u32 data_size;
+};
+
+struct imx290 {
+	struct device *dev;
+	struct clk *xclk;
+	struct regmap *regmap;
+
+	struct v4l2_subdev sd;
+	struct v4l2_fwnode_endpoint ep;
+	struct media_pad pad;
+	struct v4l2_mbus_framefmt current_format;
+	const struct imx290_mode *current_mode;
+
+	struct regulator_bulk_data supplies[IMX290_NUM_SUPPLIES];
+	struct gpio_desc *rst_gpio;
+
+	struct v4l2_ctrl_handler ctrls;
+	struct v4l2_ctrl *link_freq;
+	struct v4l2_ctrl *pixel_rate;
+
+	struct mutex lock;
+};
+
+struct imx290_pixfmt {
+	u32 code;
+	u32 colorspace;
+};
+
+static const struct imx290_pixfmt imx290_formats[] = {
+	{ MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_COLORSPACE_SRGB, },
+};
+
+static struct regmap_config imx290_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static const struct imx290_regval imx290_global_init_settings[] = {
+	{ 0x3007, 0x00 },
+	{ 0x3009, 0x00 },
+	{ 0x3018, 0x65 },
+	{ 0x3019, 0x04 },
+	{ 0x301a, 0x00 },
+	{ 0x3443, 0x03 },
+	{ 0x3444, 0x20 },
+	{ 0x3445, 0x25 },
+	{ 0x3407, 0x03 },
+	{ 0x303a, 0x0c },
+	{ 0x3040, 0x00 },
+	{ 0x3041, 0x00 },
+	{ 0x303c, 0x00 },
+	{ 0x303d, 0x00 },
+	{ 0x3042, 0x9c },
+	{ 0x3043, 0x07 },
+	{ 0x303e, 0x49 },
+	{ 0x303f, 0x04 },
+	{ 0x304b, 0x0a },
+	{ 0x300f, 0x00 },
+	{ 0x3010, 0x21 },
+	{ 0x3012, 0x64 },
+	{ 0x3016, 0x09 },
+	{ 0x3070, 0x02 },
+	{ 0x3071, 0x11 },
+	{ 0x309b, 0x10 },
+	{ 0x309c, 0x22 },
+	{ 0x30a2, 0x02 },
+	{ 0x30a6, 0x20 },
+	{ 0x30a8, 0x20 },
+	{ 0x30aa, 0x20 },
+	{ 0x30ac, 0x20 },
+	{ 0x30b0, 0x43 },
+	{ 0x3119, 0x9e },
+	{ 0x311c, 0x1e },
+	{ 0x311e, 0x08 },
+	{ 0x3128, 0x05 },
+	{ 0x313d, 0x83 },
+	{ 0x3150, 0x03 },
+	{ 0x317e, 0x00 },
+	{ 0x32b8, 0x50 },
+	{ 0x32b9, 0x10 },
+	{ 0x32ba, 0x00 },
+	{ 0x32bb, 0x04 },
+	{ 0x32c8, 0x50 },
+	{ 0x32c9, 0x10 },
+	{ 0x32ca, 0x00 },
+	{ 0x32cb, 0x04 },
+	{ 0x332c, 0xd3 },
+	{ 0x332d, 0x10 },
+	{ 0x332e, 0x0d },
+	{ 0x3358, 0x06 },
+	{ 0x3359, 0xe1 },
+	{ 0x335a, 0x11 },
+	{ 0x3360, 0x1e },
+	{ 0x3361, 0x61 },
+	{ 0x3362, 0x10 },
+	{ 0x33b0, 0x50 },
+	{ 0x33b2, 0x1a },
+	{ 0x33b3, 0x04 },
+};
+
+static const struct imx290_regval imx290_1080p_settings[] = {
+	/* mode settings */
+	{ 0x3007, 0x00 },
+	{ 0x303a, 0x0c },
+	{ 0x3414, 0x0a },
+	{ 0x3472, 0x80 },
+	{ 0x3473, 0x07 },
+	{ 0x3418, 0x38 },
+	{ 0x3419, 0x04 },
+	{ 0x3012, 0x64 },
+	{ 0x3013, 0x00 },
+	{ 0x305c, 0x18 },
+	{ 0x305d, 0x03 },
+	{ 0x305e, 0x20 },
+	{ 0x305f, 0x01 },
+	{ 0x315e, 0x1a },
+	{ 0x3164, 0x1a },
+	{ 0x3480, 0x49 },
+	/* data rate settings */
+	{ 0x3009, 0x01 },
+	{ 0x3405, 0x10 },
+	{ 0x3446, 0x57 },
+	{ 0x3447, 0x00 },
+	{ 0x3448, 0x37 },
+	{ 0x3449, 0x00 },
+	{ 0x344a, 0x1f },
+	{ 0x344b, 0x00 },
+	{ 0x344c, 0x1f },
+	{ 0x344d, 0x00 },
+	{ 0x344e, 0x1f },
+	{ 0x344f, 0x00 },
+	{ 0x3450, 0x77 },
+	{ 0x3451, 0x00 },
+	{ 0x3452, 0x1f },
+	{ 0x3453, 0x00 },
+	{ 0x3454, 0x17 },
+	{ 0x3455, 0x00 },
+	{ 0x301c, 0x98 },
+	{ 0x301d, 0x08 },
+};
+
+static const struct imx290_regval imx290_720p_settings[] = {
+	/* mode settings */
+	{ 0x3007, 0x10 },
+	{ 0x303a, 0x06 },
+	{ 0x3414, 0x04 },
+	{ 0x3472, 0x00 },
+	{ 0x3473, 0x05 },
+	{ 0x3418, 0xd0 },
+	{ 0x3419, 0x02 },
+	{ 0x3012, 0x64 },
+	{ 0x3013, 0x00 },
+	{ 0x305c, 0x20 },
+	{ 0x305d, 0x00 },
+	{ 0x305e, 0x20 },
+	{ 0x305f, 0x01 },
+	{ 0x315e, 0x1a },
+	{ 0x3164, 0x1a },
+	{ 0x3480, 0x49 },
+	/* data rate settings */
+	{ 0x3009, 0x01 },
+	{ 0x3405, 0x10 },
+	{ 0x3446, 0x4f },
+	{ 0x3447, 0x00 },
+	{ 0x3448, 0x2f },
+	{ 0x3449, 0x00 },
+	{ 0x344a, 0x17 },
+	{ 0x344b, 0x00 },
+	{ 0x344c, 0x17 },
+	{ 0x344d, 0x00 },
+	{ 0x344e, 0x17 },
+	{ 0x344f, 0x00 },
+	{ 0x3450, 0x57 },
+	{ 0x3451, 0x00 },
+	{ 0x3452, 0x17 },
+	{ 0x3453, 0x00 },
+	{ 0x3454, 0x17 },
+	{ 0x3455, 0x00 },
+	{ 0x301c, 0xe4 },
+	{ 0x301d, 0x0c },
+};
+
+static const struct imx290_regval imx290_10bit_settings[] = {
+	{ 0x3005, 0x00},
+	{ 0x3046, 0x00},
+	{ 0x3129, 0x1d},
+	{ 0x317c, 0x12},
+	{ 0x31ec, 0x37},
+	{ 0x3441, 0x0a},
+	{ 0x3442, 0x0a},
+	{ 0x300a, 0x3c},
+	{ 0x300b, 0x00},
+};
+
+/* supported link frequencies */
+static const s64 imx290_link_freq[] = {
+	445500000,
+};
+
+/* Mode configs */
+static const struct imx290_mode imx290_modes[] = {
+	{
+		.width = 1920,
+		.height = 1080,
+		.data = imx290_1080p_settings,
+		.data_size = ARRAY_SIZE(imx290_1080p_settings),
+		.pixel_rate = 178200000,
+		.link_freq_index = 0,
+	},
+	{
+		.width = 1280,
+		.height = 720,
+		.data = imx290_720p_settings,
+		.data_size = ARRAY_SIZE(imx290_720p_settings),
+		.pixel_rate = 178200000,
+		.link_freq_index = 0,
+	},
+};
+
+static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
+{
+	return container_of(_sd, struct imx290, sd);
+}
+
+static inline int imx290_read_reg(struct imx290 *imx290, u16 addr, u8 *value)
+{
+	u32 regval;
+	int ret;
+
+	ret = regmap_read(imx290->regmap, addr, &regval);
+	if (ret) {
+		dev_err(imx290->dev, "I2C read failed for addr: %x\n", addr);
+		return ret;
+	}
+
+	*value = regval & 0xFF;
+
+	return 0;
+}
+
+static int imx290_write_reg(struct imx290 *imx290, u16 addr, u8 value)
+{
+	int ret;
+
+	ret = regmap_write(imx290->regmap, addr, value);
+	if (ret) {
+		dev_err(imx290->dev, "I2C write failed for addr: %x\n", addr);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int imx290_set_register_array(struct imx290 *imx290,
+				     const struct imx290_regval *settings,
+				     unsigned int num_settings)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_settings; ++i, ++settings) {
+		ret = imx290_write_reg(imx290, settings->reg, settings->val);
+		if (ret < 0)
+			return ret;
+
+		/* Settle time is 10ms for all registers */
+		msleep(10);
+	}
+
+	return 0;
+}
+
+static int imx290_write_buffered_reg(struct imx290 *imx290, u16 address_low,
+				     u8 nr_regs, u32 value)
+{
+	int ret, i;
+	u8 val = 0;
+
+	ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x01);
+	if (ret) {
+		dev_err(imx290->dev, "Error setting hold register\n");
+		return ret;
+	}
+
+	for (i = 0; i < nr_regs; i++) {
+		imx290_read_reg(imx290, address_low + i, &val);
+		ret = imx290_write_reg(imx290, address_low + i,
+				       (u8)(value >> (i * 8)));
+		if (ret) {
+			dev_err(imx290->dev, "Error writing buffered registers\n");
+			return ret;
+		}
+		imx290_read_reg(imx290, address_low + i, &val);
+	}
+
+	ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x00);
+	if (ret) {
+		dev_err(imx290->dev, "Error setting hold register\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int imx290_set_gain(struct imx290 *imx290, u32 value)
+{
+	int ret;
+
+	u32 adjusted_value = (value * 10) / 3;
+
+	ret = imx290_write_buffered_reg(imx290, IMX290_GAIN, 1, adjusted_value);
+	if (ret)
+		dev_err(imx290->dev, "Unable to write gain\n");
+
+	return ret;
+}
+
+static int imx290_set_power_on(struct imx290 *imx290)
+{
+	int ret;
+
+	ret = clk_prepare_enable(imx290->xclk);
+	if (ret) {
+		dev_err(imx290->dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	ret = regulator_bulk_enable(IMX290_NUM_SUPPLIES,
+				    imx290->supplies);
+	if (ret) {
+		dev_err(imx290->dev, "Failed to enable regulators\n");
+		goto xclk_off;
+	}
+
+	usleep_range(1, 2);
+	gpiod_set_value_cansleep(imx290->rst_gpio, 1);
+	usleep_range(30000, 31000);
+
+	return 0;
+
+xclk_off:
+	clk_disable_unprepare(imx290->xclk);
+	return ret;
+}
+
+/* Stop streaming */
+static int imx290_stop_streaming(struct imx290 *imx290)
+{
+	int ret;
+
+	ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x01);
+	if (ret < 0)
+		return ret;
+
+	msleep(30);
+
+	return imx290_write_reg(imx290, IMX290_XMSTA, 0x01);
+}
+
+static void imx290_set_power_off(struct imx290 *imx290)
+{
+	clk_disable_unprepare(imx290->xclk);
+	gpiod_set_value_cansleep(imx290->rst_gpio, 0);
+	regulator_bulk_disable(IMX290_NUM_SUPPLIES, imx290->supplies);
+}
+
+static int imx290_s_power(struct v4l2_subdev *sd, int on)
+{
+	struct imx290 *imx290 = to_imx290(sd);
+	int ret = 0;
+
+	mutex_lock(&imx290->lock);
+
+	if (on) {
+		ret = imx290_set_power_on(imx290);
+		if (ret < 0)
+			goto exit;
+
+		ret = imx290_set_register_array(imx290,
+				imx290_global_init_settings,
+				ARRAY_SIZE(imx290_global_init_settings));
+		if (ret < 0) {
+			dev_err(imx290->dev,
+				"Could not set init registers\n");
+			imx290_set_power_off(imx290);
+			goto exit;
+		}
+
+		imx290_stop_streaming(imx290);
+	} else {
+		imx290_set_power_off(imx290);
+	}
+
+exit:
+	mutex_unlock(&imx290->lock);
+
+	return ret;
+}
+
+static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct imx290 *imx290 = container_of(ctrl->handler,
+					     struct imx290, ctrls);
+	int ret = 0;
+
+	mutex_lock(&imx290->lock);
+
+	switch (ctrl->id) {
+	case V4L2_CID_GAIN:
+		ret = imx290_set_gain(imx290, ctrl->val);
+		break;
+	default:
+		dev_info(imx290->dev, "ctrl(id:0x%x,val:0x%x) is not handled",
+			 ctrl->id, ctrl->val);
+		break;
+	}
+
+	mutex_unlock(&imx290->lock);
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops imx290_ctrl_ops = {
+	.s_ctrl = imx290_set_ctrl,
+};
+
+static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_pad_config *cfg,
+				 struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->index >= ARRAY_SIZE(imx290_formats))
+		return -EINVAL;
+
+	code->code = imx290_formats[code->index].code;
+
+	return 0;
+}
+
+static int imx290_get_fmt(struct v4l2_subdev *sd,
+			  struct v4l2_subdev_pad_config *cfg,
+			  struct v4l2_subdev_format *fmt)
+{
+	struct imx290 *imx290 = to_imx290(sd);
+	struct v4l2_mbus_framefmt *framefmt;
+
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+		framefmt = v4l2_subdev_get_try_format(&imx290->sd, cfg,
+						      fmt->pad);
+	else
+		framefmt = &imx290->current_format;
+
+	fmt->format = *framefmt;
+
+	return 0;
+}
+
+static int imx290_set_fmt(struct v4l2_subdev *sd,
+			  struct v4l2_subdev_pad_config *cfg,
+		      struct v4l2_subdev_format *fmt)
+{
+	struct imx290 *imx290 = to_imx290(sd);
+	const struct imx290_mode *mode;
+	struct v4l2_mbus_framefmt *format;
+	int i, ret = 0;
+
+	mode = v4l2_find_nearest_size(imx290_modes,
+				      ARRAY_SIZE(imx290_modes),
+				      width, height,
+				      fmt->format.width, fmt->format.height);
+
+	fmt->format.width = mode->width;
+	fmt->format.height = mode->height;
+
+	for (i = 0; i < ARRAY_SIZE(imx290_formats); i++)
+		if (imx290_formats[i].code == fmt->format.code)
+			break;
+
+	if (i >= ARRAY_SIZE(imx290_formats))
+		i = 0;
+
+	fmt->format.code = imx290_formats[i].code;
+	fmt->format.colorspace = imx290_formats[i].colorspace;
+	fmt->format.field = V4L2_FIELD_NONE;
+
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+		format = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
+	} else {
+		format = &imx290->current_format;
+		__v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
+		__v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, mode->pixel_rate);
+
+		imx290->current_mode = mode;
+	}
+
+	*format = fmt->format;
+
+	return ret;
+}
+
+static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
+				  struct v4l2_subdev_pad_config *cfg)
+{
+	struct v4l2_subdev_format fmt = { 0 };
+
+	fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
+	fmt.format.width = 1920;
+	fmt.format.height = 1080;
+
+	imx290_set_fmt(subdev, cfg, &fmt);
+
+	return 0;
+}
+
+static int imx290_write_current_format(struct imx290 *imx290,
+				       struct v4l2_mbus_framefmt *format)
+{
+	int ret;
+
+	switch (format->code) {
+	case MEDIA_BUS_FMT_SRGGB10_1X10:
+		ret = imx290_set_register_array(imx290, imx290_10bit_settings,
+					ARRAY_SIZE(imx290_10bit_settings));
+		if (ret < 0) {
+			dev_err(imx290->dev, "Could not set format registers\n");
+			return ret;
+		}
+		break;
+	default:
+		dev_err(imx290->dev, "Unknown pixel format\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Start streaming */
+static int imx290_start_streaming(struct imx290 *imx290)
+{
+	int ret;
+
+	/* Set current frame format */
+	ret = imx290_write_current_format(imx290, &imx290->current_format);
+	if (ret < 0) {
+		dev_err(imx290->dev, "Could not set frame format\n");
+		return ret;
+	}
+
+	/* Apply default values of current mode */
+	ret = imx290_set_register_array(imx290, imx290->current_mode->data,
+					imx290->current_mode->data_size);
+	if (ret < 0) {
+		dev_err(imx290->dev, "Could not set current mode\n");
+		return ret;
+	}
+
+	/* Apply customized values from user */
+	ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
+	if (ret) {
+		dev_err(imx290->dev, "Could not sync v4l2 controls\n");
+		return ret;
+	}
+
+	ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x00);
+	if (ret < 0)
+		return ret;
+
+	msleep(30);
+
+	/* Start streaming */
+	return imx290_write_reg(imx290, IMX290_XMSTA, 0x00);
+}
+
+static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct imx290 *imx290 = to_imx290(sd);
+	int ret = 0;
+
+	if (enable)
+		ret = imx290_start_streaming(imx290);
+	else
+		imx290_stop_streaming(imx290);
+
+	return ret;
+}
+
+static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
+{
+	unsigned int i;
+
+	for (i = 0; i < IMX290_NUM_SUPPLIES; i++)
+		imx290->supplies[i].supply = imx290_supply_name[i];
+
+	return devm_regulator_bulk_get(dev, IMX290_NUM_SUPPLIES,
+				       imx290->supplies);
+}
+
+static const struct v4l2_subdev_core_ops imx290_subdev_core_ops = {
+	.s_power = imx290_s_power,
+};
+
+static const struct v4l2_subdev_video_ops imx290_video_ops = {
+	.s_stream = imx290_set_stream,
+};
+
+static const struct v4l2_subdev_pad_ops imx290_pad_ops = {
+	.init_cfg = imx290_entity_init_cfg,
+	.enum_mbus_code = imx290_enum_mbus_code,
+	.get_fmt = imx290_get_fmt,
+	.set_fmt = imx290_set_fmt,
+};
+
+static const struct v4l2_subdev_ops imx290_subdev_ops = {
+	.core = &imx290_subdev_core_ops,
+	.video = &imx290_video_ops,
+	.pad = &imx290_pad_ops,
+};
+
+static const struct media_entity_operations imx290_subdev_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static int imx290_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct fwnode_handle *endpoint;
+	struct imx290 *imx290;
+	u32 xclk_freq;
+	int ret;
+
+	imx290 = devm_kzalloc(dev, sizeof(*imx290), GFP_KERNEL);
+	if (!imx290)
+		return -ENOMEM;
+
+	imx290->dev = dev;
+	imx290->regmap = devm_regmap_init_i2c(client, &imx290_regmap_config);
+	if (IS_ERR(imx290->regmap)) {
+		dev_err(dev, "Unable to initialize I2C\n");
+		return -ENODEV;
+	}
+
+	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
+	if (!endpoint) {
+		dev_err(dev, "Endpoint node not found\n");
+		return -EINVAL;
+	}
+
+	ret = v4l2_fwnode_endpoint_parse(endpoint, &imx290->ep);
+	fwnode_handle_put(endpoint);
+	if (ret < 0) {
+		dev_err(dev, "Parsing endpoint node failed\n");
+		return ret;
+	}
+
+	/* Only CSI2 is supported for now */
+	if (imx290->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
+		dev_err(dev, "Unsupported bus type, should be CSI2\n");
+		return -EINVAL;
+	}
+
+	/* Set default mode to max resolution */
+	imx290->current_mode = &imx290_modes[0];
+
+	/* get system clock (xclk) */
+	imx290->xclk = devm_clk_get(dev, "xclk");
+	if (IS_ERR(imx290->xclk)) {
+		dev_err(dev, "Could not get xclk");
+		return PTR_ERR(imx290->xclk);
+	}
+
+	ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
+	if (ret) {
+		dev_err(dev, "Could not get xclk frequency\n");
+		return ret;
+	}
+
+	/* external clock must be 37.125 MHz */
+	if (xclk_freq != 37125000) {
+		dev_err(dev, "External clock frequency %u is not supported\n",
+			xclk_freq);
+		return -EINVAL;
+	}
+
+	ret = clk_set_rate(imx290->xclk, xclk_freq);
+	if (ret) {
+		dev_err(dev, "Could not set xclk frequency\n");
+		return ret;
+	}
+
+	ret = imx290_get_regulators(dev, imx290);
+	if (ret < 0) {
+		dev_err(dev, "Cannot get regulators\n");
+		return ret;
+	}
+
+	imx290->rst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
+	if (IS_ERR(imx290->rst_gpio)) {
+		dev_err(dev, "Cannot get reset gpio\n");
+		return PTR_ERR(imx290->rst_gpio);
+	}
+
+	mutex_init(&imx290->lock);
+
+	v4l2_ctrl_handler_init(&imx290->ctrls, 3);
+
+	v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+			  V4L2_CID_GAIN, 0, 72, 1, 0);
+	imx290->link_freq = v4l2_ctrl_new_int_menu(&imx290->ctrls,
+					&imx290_ctrl_ops,
+					V4L2_CID_LINK_FREQ,
+					ARRAY_SIZE(imx290_link_freq) - 1,
+					0, imx290_link_freq);
+	if (imx290->link_freq)
+		imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+					       V4L2_CID_PIXEL_RATE, 1,
+					       INT_MAX, 1,
+					       imx290_modes[0].pixel_rate);
+
+	imx290->sd.ctrl_handler = &imx290->ctrls;
+
+	if (imx290->ctrls.error) {
+		dev_err(dev, "Control initialization error %d\n",
+			imx290->ctrls.error);
+		ret = imx290->ctrls.error;
+		goto free_ctrl;
+	}
+
+	v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops);
+	imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	imx290->sd.dev = &client->dev;
+	imx290->sd.entity.ops = &imx290_subdev_entity_ops;
+	imx290->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+	imx290->pad.flags = MEDIA_PAD_FL_SOURCE;
+	ret = media_entity_pads_init(&imx290->sd.entity, 1, &imx290->pad);
+	if (ret < 0) {
+		dev_err(dev, "Could not register media entity\n");
+		goto free_ctrl;
+	}
+
+	ret = v4l2_async_register_subdev(&imx290->sd);
+	if (ret < 0) {
+		dev_err(dev, "Could not register v4l2 device\n");
+		goto free_entity;
+	}
+
+	return 0;
+
+free_entity:
+	media_entity_cleanup(&imx290->sd.entity);
+free_ctrl:
+	v4l2_ctrl_handler_free(&imx290->ctrls);
+	mutex_destroy(&imx290->lock);
+
+	return ret;
+}
+
+static int imx290_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct imx290 *imx290 = to_imx290(sd);
+
+	v4l2_async_unregister_subdev(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
+
+	imx290_set_power_off(imx290);
+	mutex_destroy(&imx290->lock);
+
+	return 0;
+}
+
+static const struct of_device_id imx290_of_match[] = {
+	{ .compatible = "sony,imx290" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx290_of_match);
+
+static struct i2c_driver imx290_i2c_driver = {
+	.probe  = imx290_probe,
+	.remove = imx290_remove,
+	.driver = {
+		.name  = "imx290",
+		.of_match_table = of_match_ptr(imx290_of_match),
+	},
+};
+
+module_i2c_driver(imx290_i2c_driver);
+
+MODULE_DESCRIPTION("Sony IMX290 CMOS Image Sensor Driver");
+MODULE_AUTHOR("FRAMOS GmbH");
+MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 1/3] dt-bindings: media: i2c: Add IMX290 CMOS sensor binding
From: Manivannan Sadhasivam @ 2019-08-06 13:09 UTC (permalink / raw)
  To: mchehab, robh+dt
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, Manivannan Sadhasivam
In-Reply-To: <20190806130938.19916-1-manivannan.sadhasivam@linaro.org>

Add devicetree binding for IMX290 CMOS image sensor.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/media/i2c/imx290.txt  | 51 +++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx290.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/imx290.txt b/Documentation/devicetree/bindings/media/i2c/imx290.txt
new file mode 100644
index 000000000000..7535b5b5b24b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx290.txt
@@ -0,0 +1,51 @@
+* Sony IMX290 1/2.8-Inch CMOS Image Sensor
+
+The Sony IMX290 is a 1/2.8-Inch CMOS Solid-state image sensor with
+Square Pixel for Color Cameras. It is programmable through I2C and 4-wire
+interfaces. The sensor output is available via CMOS logic parallel SDR output,
+Low voltage LVDS DDR output and CSI-2 serial data output.
+
+Required Properties:
+- compatible: Should be "sony,imx290"
+- reg: I2C bus address of the device
+- clocks: Reference to the xclk clock.
+- clock-names: Should be "xclk".
+- clock-frequency: Frequency of the xclk clock.
+- vdddo-supply: Sensor digital IO regulator.
+- vdda-supply: Sensor analog regulator.
+- vddd-supply: Sensor digital core regulator.
+
+Optional Properties:
+- reset-gpios: Sensor reset GPIO
+
+The imx290 device node should contain one 'port' child node with
+an 'endpoint' subnode. For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+	&i2c1 {
+		...
+		imx290: imx290@1a {
+			compatible = "sony,imx290";
+			reg = <0x1a>;
+
+			reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&camera_rear_default>;
+
+			clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
+			clock-names = "xclk";
+			clock-frequency = <37125000>;
+
+			vdddo-supply = <&camera_vdddo_1v8>;
+			vdda-supply = <&camera_vdda_2v8>;
+			vddd-supply = <&camera_vddd_1v5>;
+
+			port {
+				imx290_ep: endpoint {
+					clock-lanes = <1>;
+					data-lanes = <0 2 3 4>;
+					remote-endpoint = <&csiphy0_ep>;
+				};
+			};
+		};
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 0/3] Add IMX290 CMOS image sensor support
From: Manivannan Sadhasivam @ 2019-08-06 13:09 UTC (permalink / raw)
  To: mchehab, robh+dt
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, Manivannan Sadhasivam

Hello,

This patchset adds support for IMX290 CMOS image sensor from Sony.
Sensor can be programmed through I2C and 4-wire interface but the
current driver only supports I2C interface. Also, the sensor is
capable of outputting frames in following 3 interfaces:

* CMOS logic parallel SDR output
* Low voltage LVDS serial DDR output
* CSI-2 serial data output

But the current driver only supports CSI-2 output available from 4 lanes.
In the case of sensor resolution, driver only supports 1920x1080 and
1280x720 at mid data rate of 445.5 Mpbs.

The driver has been validated using Framos IMX290 module interfaced to
96Boards Dragonboard410c.

Thanks,
Mani

Changes in v2:

* Added Reviewed-by tag from Rob for bindings patch

Manivannan Sadhasivam (3):
  dt-bindings: media: i2c: Add IMX290 CMOS sensor binding
  media: i2c: Add IMX290 CMOS image sensor driver
  MAINTAINERS: Add entry for IMX290 CMOS image sensor driver

 .../devicetree/bindings/media/i2c/imx290.txt  |  51 ++
 MAINTAINERS                                   |   8 +
 drivers/media/i2c/Kconfig                     |  11 +
 drivers/media/i2c/Makefile                    |   1 +
 drivers/media/i2c/imx290.c                    | 845 ++++++++++++++++++
 5 files changed, 916 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx290.txt
 create mode 100644 drivers/media/i2c/imx290.c

-- 
2.17.1

^ permalink raw reply

* [PATCH v3 6/6] MAINTAINERS: add entry for Amlogic Thermal driver
From: Guillaume La Roque @ 2019-08-06 13:05 UTC (permalink / raw)
  To: daniel.lezcano, khilman
  Cc: linux-pm, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190806130506.8753-1-glaroque@baylibre.com>

Add myself as maintainer for Amlogic Thermal driver.

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fb2b12f75c37..299f27d11058 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15910,6 +15910,15 @@ F:	Documentation/driver-api/thermal/cpu-cooling-api.rst
 F:	drivers/thermal/cpu_cooling.c
 F:	include/linux/cpu_cooling.h
 
+THERMAL DRIVER FOR AMLOGIC SOCS
+M:	Guillaume La Roque <glaroque@baylibre.com>
+L:	linux-pm@vger.kernel.org
+L:	linux-amlogic@lists.infradead.org
+W:	http://linux-meson.com/
+S:	Supported
+F:	drivers/thermal/amlogic_thermal.c
+F:	Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
+
 THINKPAD ACPI EXTRAS DRIVER
 M:	Henrique de Moraes Holschuh <ibm-acpi@hmh.eng.br>
 L:	ibm-acpi-devel@lists.sourceforge.net
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 5/6] arm64: dts: amlogic: odroid-n2: add minimal thermal zone
From: Guillaume La Roque @ 2019-08-06 13:05 UTC (permalink / raw)
  To: daniel.lezcano, khilman
  Cc: linux-pm, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190806130506.8753-1-glaroque@baylibre.com>

Add minimal thermal zone for two temperature sensor
One is located close to the DDR and the other one is
located close to the PLLs (between the CPU and GPU)

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 .../boot/dts/amlogic/meson-g12b-odroid-n2.dts | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
index 75ff8a7e373d..a7d73c0c8447 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
@@ -10,6 +10,7 @@
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/meson-g12a-gpio.h>
 #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	compatible = "hardkernel,odroid-n2", "amlogic,g12b";
@@ -20,6 +21,55 @@
 		ethernet0 = &ethmac;
 	};
 
+	thermal-zones {
+		cpu-thermal {
+			polling-delay = <1000>;
+			polling-delay-passive = <100>;
+			thermal-sensors = <&cpu_temp>;
+
+			trips {
+				cpu_critical: cpu-critical {
+					temperature = <110000>; /* millicelsius */
+					hysteresis = <2000>; /* millicelsius */
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map {
+					trip = <&cpu_critical>;
+					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu100 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu101 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu102 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu103 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+
+		ddr-thermal {
+			polling-delay = <1000>;
+			polling-delay-passive = <100>;
+			thermal-sensors = <&ddr_temp>;
+
+			trips {
+				ddr_critical: ddr-critical {
+					temperature = <110000>; /* millicelsius */
+					hysteresis = <2000>; /* millicelsius */
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map {
+					trip = <&ddr_critical>;
+					cooling-device = <&mali THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
@@ -288,6 +338,7 @@
 	operating-points-v2 = <&cpu_opp_table_0>;
 	clocks = <&clkc CLKID_CPU_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu1 {
@@ -295,6 +346,7 @@
 	operating-points-v2 = <&cpu_opp_table_0>;
 	clocks = <&clkc CLKID_CPU_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu100 {
@@ -302,6 +354,7 @@
 	operating-points-v2 = <&cpub_opp_table_1>;
 	clocks = <&clkc CLKID_CPUB_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu101 {
@@ -309,6 +362,7 @@
 	operating-points-v2 = <&cpub_opp_table_1>;
 	clocks = <&clkc CLKID_CPUB_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu102 {
@@ -316,6 +370,7 @@
 	operating-points-v2 = <&cpub_opp_table_1>;
 	clocks = <&clkc CLKID_CPUB_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu103 {
@@ -323,6 +378,7 @@
 	operating-points-v2 = <&cpub_opp_table_1>;
 	clocks = <&clkc CLKID_CPUB_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &ext_mdio {
@@ -377,6 +433,10 @@
 	};
 };
 
+&mali {
+	#cooling-cells = <2>;
+};
+
 &hdmi_tx {
 	status = "okay";
 	pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 4/6] arm64: dts: meson: sei510: Add minimal thermal zone
From: Guillaume La Roque @ 2019-08-06 13:05 UTC (permalink / raw)
  To: daniel.lezcano, khilman
  Cc: linux-pm, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190806130506.8753-1-glaroque@baylibre.com>

Add minimal thermal zone for two temperature sensor
One is located close to the DDR and the other one is
located close to the PLLs (between the CPU and GPU)

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 .../boot/dts/amlogic/meson-g12a-sei510.dts    | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
index 979449968a5f..2c16a2cba0a3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
@@ -10,6 +10,7 @@
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/meson-g12a-gpio.h>
 #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	compatible = "seirobotics,sei510", "amlogic,g12a";
@@ -33,6 +34,53 @@
 		ethernet0 = &ethmac;
 	};
 
+	thermal-zones {
+		cpu-thermal {
+			polling-delay = <1000>;
+			polling-delay-passive = <100>;
+			thermal-sensors = <&cpu_temp>;
+
+			trips {
+				cpu_critical: cpu-critical {
+					temperature = <110000>; /* millicelsius */
+					hysteresis = <2000>; /* millicelsius */
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map {
+					trip = <&cpu_critical>;
+					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+
+		ddr-thermal {
+			polling-delay = <1000>;
+			polling-delay-passive = <100>;
+			thermal-sensors = <&ddr_temp>;
+
+			trips {
+				ddr_critical: ddr-critical {
+					temperature = <110000>; /* millicelsius */
+					hysteresis = <2000>; /* millicelsius */
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map {
+					trip = <&ddr_critical>;
+					cooling-device = <&mali THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+	};
+
 	mono_dac: audio-codec-0 {
 		compatible = "maxim,max98357a";
 		#sound-dai-cells = <0>;
@@ -321,6 +369,7 @@
 	operating-points-v2 = <&cpu_opp_table>;
 	clocks = <&clkc CLKID_CPU_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu1 {
@@ -328,6 +377,7 @@
 	operating-points-v2 = <&cpu_opp_table>;
 	clocks = <&clkc CLKID_CPU_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu2 {
@@ -335,6 +385,7 @@
 	operating-points-v2 = <&cpu_opp_table>;
 	clocks = <&clkc CLKID_CPU_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cpu3 {
@@ -342,6 +393,7 @@
 	operating-points-v2 = <&cpu_opp_table>;
 	clocks = <&clkc CLKID_CPU_CLK>;
 	clock-latency = <50000>;
+	#cooling-cells = <2>;
 };
 
 &cvbs_vdac_port {
@@ -368,6 +420,10 @@
 	status = "okay";
 };
 
+&mali {
+	#cooling-cells = <2>;
+};
+
 &hdmi_tx {
 	status = "okay";
 	pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 3/6] arm64: dts: amlogic: g12: add temperature sensor
From: Guillaume La Roque @ 2019-08-06 13:05 UTC (permalink / raw)
  To: daniel.lezcano, khilman
  Cc: linux-pm, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190806130506.8753-1-glaroque@baylibre.com>

Add cpu and ddr temperature sensors for G12 Socs

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../boot/dts/amlogic/meson-g12-common.dtsi    | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 06e186ca41e3..a06298538614 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1353,6 +1353,26 @@
 				};
 			};
 
+			cpu_temp: temperature-sensor@34800 {
+				compatible = "amlogic,g12-cpu-thermal",
+					     "amlogic,g12-thermal";
+				reg = <0x0 0x34800 0x0 0x50>;
+				interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>;
+				clocks = <&clkc CLKID_TS>;
+				#thermal-sensor-cells = <0>;
+				amlogic,ao-secure = <&sec_AO>;
+			};
+
+			ddr_temp: temperature-sensor@34c00 {
+				compatible = "amlogic,g12-ddr-thermal",
+					     "amlogic,g12-thermal";
+				reg = <0x0 0x34c00 0x0 0x50>;
+				interrupts = <GIC_SPI 36 IRQ_TYPE_EDGE_RISING>;
+				clocks = <&clkc CLKID_TS>;
+				#thermal-sensor-cells = <0>;
+				amlogic,ao-secure = <&sec_AO>;
+			};
+
 			usb2_phy0: phy@36000 {
 				compatible = "amlogic,g12a-usb2-phy";
 				reg = <0x0 0x36000 0x0 0x2000>;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 2/6] thermal: amlogic: Add thermal driver to support G12 SoCs
From: Guillaume La Roque @ 2019-08-06 13:05 UTC (permalink / raw)
  To: daniel.lezcano, khilman
  Cc: linux-pm, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190806130506.8753-1-glaroque@baylibre.com>

Amlogic G12A and G12B SoCs integrate two thermal sensors
with the same design.
One is located close to the DDR controller and the other one is
located close to the PLLs (between the CPU and GPU).

The calibration data for each of the thermal sensors instance is
stored in a different location within the AO region.

Implement reading the temperature from each thermal sensor.

The IP block has more functionality, which may be added to this driver
in the future:
- chip reset when the temperature exceeds a configurable threshold
- up to four interrupts when the temperature has risen above a
configurable threshold
- up to four interrupts when the temperature has fallen below a
configurable threshold

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 drivers/thermal/Kconfig           |  11 +
 drivers/thermal/Makefile          |   1 +
 drivers/thermal/amlogic_thermal.c | 336 ++++++++++++++++++++++++++++++
 3 files changed, 348 insertions(+)
 create mode 100644 drivers/thermal/amlogic_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 9966364a6deb..0f31bb4bc372 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -348,6 +348,17 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+config AMLOGIC_THERMAL
+	tristate "Amlogic Thermal Support"
+	default ARCH_MESON
+	depends on OF && ARCH_MESON
+	help
+	  If you say yes here you get support for Amlogic Thermal
+	  for G12 SoC Family.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called amlogic_thermal.
+
 menu "Intel thermal drivers"
 depends on X86 || X86_INTEL_QUARK || COMPILE_TEST
 source "drivers/thermal/intel/Kconfig"
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 74a37c7f847a..baeb70bf0568 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -54,3 +54,4 @@ obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o
 obj-$(CONFIG_GENERIC_ADC_THERMAL)	+= thermal-generic-adc.o
 obj-$(CONFIG_ZX2967_THERMAL)	+= zx2967_thermal.o
 obj-$(CONFIG_UNIPHIER_THERMAL)	+= uniphier_thermal.o
+obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
new file mode 100644
index 000000000000..672d11abd8c7
--- /dev/null
+++ b/drivers/thermal/amlogic_thermal.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Thermal Sensor Driver
+ *
+ * Copyright (C) 2017 Huan Biao <huan.biao@amlogic.com>
+ * Copyright (C) 2019 Guillaume La Roque <glaroque@baylibre.com>
+ *
+ * Register value to celsius temperature formulas:
+ *	Read_Val	    m * U
+ * U = ---------, Uptat = ---------
+ *	2^16		  1 + n * U
+ *
+ * Temperature = A * ( Uptat + u_efuse / 2^16 )- B
+ *
+ *  A B m n : calibration parameters
+ *  u_efuse : fused calibration value, it's a signed 16 bits value
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+#define TSENSOR_CFG_REG1			0x4
+	#define TSENSOR_CFG_REG1_RSET_VBG	BIT(12)
+	#define TSENSOR_CFG_REG1_RSET_ADC	BIT(11)
+	#define TSENSOR_CFG_REG1_VCM_EN		BIT(10)
+	#define TSENSOR_CFG_REG1_VBG_EN		BIT(9)
+	#define TSENSOR_CFG_REG1_OUT_CTL	BIT(6)
+	#define TSENSOR_CFG_REG1_FILTER_EN	BIT(5)
+	#define TSENSOR_CFG_REG1_DEM_EN		BIT(3)
+	#define TSENSOR_CFG_REG1_CH_SEL		GENMASK(1, 0)
+	#define TSENSOR_CFG_REG1_ENABLE		\
+		(TSENSOR_CFG_REG1_FILTER_EN |	\
+		 TSENSOR_CFG_REG1_VCM_EN |	\
+		 TSENSOR_CFG_REG1_VBG_EN |	\
+		 TSENSOR_CFG_REG1_DEM_EN |	\
+		 TSENSOR_CFG_REG1_CH_SEL)
+
+#define TSENSOR_STAT0			0x40
+
+#define TSENSOR_STAT9			0x64
+
+#define TSENSOR_READ_TEMP_MASK		GENMASK(15, 0)
+#define TSENSOR_TEMP_MASK		GENMASK(11, 0)
+
+#define TSENSOR_TRIM_SIGN_MASK		BIT(15)
+#define TSENSOR_TRIM_TEMP_MASK		GENMASK(14, 0)
+#define TSENSOR_TRIM_VERSION_MASK	GENMASK(31, 24)
+
+#define TSENSOR_TRIM_VERSION(_version)	\
+	FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version)
+
+#define TSENSOR_TRIM_CALIB_VALID_MASK	(GENMASK(3, 2) | BIT(7))
+
+#define TSENSOR_CALIB_OFFSET	1
+#define TSENSOR_CALIB_SHIFT	4
+
+/**
+ * struct amlogic_thermal_soc_calib_data
+ * @A, B, m, n: calibration parameters
+ * This structure is required for configuration of amlogic thermal driver.
+ */
+struct amlogic_thermal_soc_calib_data {
+	int A;
+	int B;
+	int m;
+	int n;
+};
+
+/**
+ * struct amlogic_thermal_data
+ * @u_efuse_off: register offset to read fused calibration value
+ * @soc: calibration parameters structure pointer
+ * @regmap_config: regmap config for the device
+ * This structure is required for configuration of amlogic thermal driver.
+ */
+struct amlogic_thermal_data {
+	int u_efuse_off;
+	const struct amlogic_thermal_soc_calib_data *calibration_parameters;
+	const struct regmap_config *regmap_config;
+};
+
+struct amlogic_thermal {
+	struct platform_device *pdev;
+	const struct amlogic_thermal_data *data;
+	struct regmap *regmap;
+	struct regmap *sec_ao_map;
+	struct clk *clk;
+	struct thermal_zone_device *tzd;
+	u32 trim_info;
+	void __iomem *base;
+};
+
+/*
+ * Calculate a temperature value from a temperature code.
+ * The unit of the temperature is degree milliCelsius.
+ */
+static int amlogic_thermal_code_to_millicelsius(struct amlogic_thermal *pdata,
+						int temp_code)
+{
+	const struct amlogic_thermal_soc_calib_data *param =
+					pdata->data->calibration_parameters;
+	int temp;
+	s64 factor, Uptat, uefuse;
+
+	uefuse = pdata->trim_info & TSENSOR_TRIM_SIGN_MASK ?
+			     ~(pdata->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
+			     (pdata->trim_info & TSENSOR_TRIM_TEMP_MASK);
+
+	factor = param->n * temp_code;
+	factor = div_s64(factor, 100);
+
+	Uptat = temp_code * param->m;
+	Uptat = div_s64(Uptat, 100);
+	Uptat = Uptat * BIT(16);
+	Uptat = div_s64(Uptat, BIT(16) + factor);
+
+	temp = (Uptat + uefuse) * param->A;
+	temp = div_s64(temp, BIT(16));
+	temp = (temp - param->B) * 100;
+
+	return temp;
+}
+
+static int amlogic_thermal_initialize(struct amlogic_thermal *pdata)
+{
+	int ret = 0;
+	int ver;
+
+	regmap_read(pdata->sec_ao_map, pdata->data->u_efuse_off,
+		    &pdata->trim_info);
+
+	ver = TSENSOR_TRIM_VERSION(pdata->trim_info);
+
+	if ((ver & TSENSOR_TRIM_CALIB_VALID_MASK) == 0) {
+		ret = -EINVAL;
+		dev_err(&pdata->pdev->dev,
+			"tsensor thermal calibration not supported: 0x%x!\n",
+			ver);
+	}
+
+	return ret;
+}
+
+static int amlogic_thermal_enable(struct amlogic_thermal *data)
+{
+	int ret;
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+	regmap_update_bits(data->regmap, TSENSOR_CFG_REG1,
+			   TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE);
+
+	return 0;
+}
+
+static int amlogic_thermal_disable(struct amlogic_thermal *data)
+{
+	regmap_update_bits(data->regmap, TSENSOR_CFG_REG1,
+			   TSENSOR_CFG_REG1_ENABLE, 0);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int amlogic_thermal_get_temp(void *data, int *temp)
+{
+	unsigned int tval;
+	struct amlogic_thermal *pdata = data;
+
+	if (!data)
+		return -EINVAL;
+
+	regmap_read(pdata->regmap, TSENSOR_STAT0, &tval);
+	*temp =
+	   amlogic_thermal_code_to_millicelsius(pdata,
+						tval & TSENSOR_READ_TEMP_MASK);
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops amlogic_thermal_ops = {
+	.get_temp	= amlogic_thermal_get_temp,
+};
+
+static const struct regmap_config amlogic_thermal_regmap_config_g12 = {
+	.reg_bits = 8,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = TSENSOR_STAT9,
+};
+
+static const struct amlogic_thermal_soc_calib_data amlogic_thermal_g12 = {
+	.A = 9411,
+	.B = 3159,
+	.m = 424,
+	.n = 324,
+};
+
+static const struct amlogic_thermal_data amlogic_thermal_g12_cpu_param = {
+	.u_efuse_off = 0x128,
+	.calibration_parameters = &amlogic_thermal_g12,
+	.regmap_config = &amlogic_thermal_regmap_config_g12,
+};
+
+static const struct amlogic_thermal_data amlogic_thermal_g12_ddr_param = {
+	.u_efuse_off = 0xf0,
+	.calibration_parameters = &amlogic_thermal_g12,
+	.regmap_config = &amlogic_thermal_regmap_config_g12,
+};
+
+static const struct of_device_id of_amlogic_thermal_match[] = {
+	{
+		.compatible = "amlogic,g12-ddr-thermal",
+		.data = &amlogic_thermal_g12_ddr_param,
+	},
+	{
+		.compatible = "amlogic,g12-cpu-thermal",
+		.data = &amlogic_thermal_g12_cpu_param,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_amlogic_thermal_match);
+
+static int amlogic_thermal_probe(struct platform_device *pdev)
+{
+	struct amlogic_thermal *pdata;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int ret;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->data = of_device_get_match_data(dev);
+	pdata->pdev = pdev;
+	platform_set_drvdata(pdev, pdata);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pdata->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pdata->base)) {
+		dev_err(dev, "failed to get io address\n");
+		return PTR_ERR(pdata->base);
+	}
+
+	pdata->regmap = devm_regmap_init_mmio(dev, pdata->base,
+					      pdata->data->regmap_config);
+	if (IS_ERR(pdata->regmap))
+		return PTR_ERR(pdata->regmap);
+
+	pdata->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(pdata->clk)) {
+		if (PTR_ERR(pdata->clk) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get clock\n");
+		return PTR_ERR(pdata->clk);
+	}
+
+	pdata->sec_ao_map = syscon_regmap_lookup_by_phandle
+		(pdev->dev.of_node, "amlogic,ao-secure");
+	if (IS_ERR(pdata->sec_ao_map)) {
+		dev_err(dev, "syscon regmap lookup failed.\n");
+		return PTR_ERR(pdata->sec_ao_map);
+	}
+
+	pdata->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
+							  0,
+							  pdata,
+							  &amlogic_thermal_ops);
+	if (IS_ERR(pdata->tzd)) {
+		ret = PTR_ERR(pdata->tzd);
+		dev_err(dev, "Failed to register tsensor: %d\n", ret);
+		return PTR_ERR(pdata->tzd);
+	}
+
+	ret = amlogic_thermal_initialize(pdata);
+	if (ret)
+		return ret;
+
+	ret = amlogic_thermal_enable(pdata);
+	if (ret)
+		clk_disable_unprepare(pdata->clk);
+
+	return ret;
+}
+
+static int amlogic_thermal_remove(struct platform_device *pdev)
+{
+	struct amlogic_thermal *data = platform_get_drvdata(pdev);
+
+	return amlogic_thermal_disable(data);
+}
+
+static int __maybe_unused amlogic_thermal_suspend(struct device *dev)
+{
+	struct amlogic_thermal *data = dev_get_drvdata(dev);
+
+	return amlogic_thermal_disable(data);
+}
+
+static int __maybe_unused amlogic_thermal_resume(struct device *dev)
+{
+	struct amlogic_thermal *data = dev_get_drvdata(dev);
+
+	return amlogic_thermal_enable(data);
+}
+
+static SIMPLE_DEV_PM_OPS(amlogic_thermal_pm_ops,
+			 amlogic_thermal_suspend, amlogic_thermal_resume);
+
+static struct platform_driver amlogic_thermal_driver = {
+	.driver = {
+		.name		= "amlogic_thermal",
+		.pm		= &amlogic_thermal_pm_ops,
+		.of_match_table = of_amlogic_thermal_match,
+	},
+	.probe	= amlogic_thermal_probe,
+	.remove	= amlogic_thermal_remove,
+};
+
+module_platform_driver(amlogic_thermal_driver);
+
+MODULE_AUTHOR("Guillaume La Roque <glaroque@baylibre.com>");
+MODULE_DESCRIPTION("Amlogic thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 1/6] dt-bindings: thermal: Add DT bindings documentation for Amlogic Thermal
From: Guillaume La Roque @ 2019-08-06 13:05 UTC (permalink / raw)
  To: daniel.lezcano, khilman
  Cc: linux-pm, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190806130506.8753-1-glaroque@baylibre.com>

Adding the devicetree binding documentation for the Amlogic temperature
sensor found in the Amlogic Meson G12 SoCs.
the G12A  and G12B SoCs are supported.

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 .../bindings/thermal/amlogic,thermal.yaml     | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml

diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
new file mode 100644
index 000000000000..d25e59113398
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/amlogic,thermal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic Thermal
+
+maintainers:
+  - Guillaume La Roque <glaroque@baylibre.com>
+
+description: Binding for Amlogic Thermal Driver
+
+properties:
+  compatible:
+      items:
+        - enum:
+            - amlogic,g12-cpu-thermal
+            - amlogic,g12-ddr-thermal
+        - const: amlogic,g12-thermal
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  amlogic,ao-secure:
+    description: phandle to the ao-secure syscon
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - amlogic,ao-secure
+
+examples:
+  - |
+        cpu_temp: temperature-sensor@ff634800 {
+                compatible = "amlogic,g12-cpu-thermal",
+                             "amlogic,g12-thermal";
+                reg = <0xff634800 0x50>;
+                interrupts = <0x0 0x24 0x0>;
+                clocks = <&clk 164>;
+                #thermal-sensor-cells = <0>;
+                amlogic,ao-secure = <&sec_AO>;
+        };
+...
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 0/6] Add support of New Amlogic temperature sensor for G12 SoCs
From: Guillaume La Roque @ 2019-08-06 13:05 UTC (permalink / raw)
  To: daniel.lezcano, khilman
  Cc: linux-pm, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel

This patchs series add support of New Amlogic temperature sensor and minimal
thermal zone for SEI510 and ODROID-N2 boards.

First implementation was doing on IIO[1] but after comments i move on thermal framework.
Formulas and calibration values come from amlogic.

Changes since v2:
  - fix yaml documention 
  - remove unneeded status variable for temperature-sensor node
  - rework driver after Martin review
  - add some information in commit message

Changes since v1:
  - fix enum vs const in documentation
  - fix error with thermal-sensor-cells value set to 1 instead of 0
  - add some dependencies needed to add cooling-maps

Dependencies :
- patch 3,4 & 5: depends on Neil's patch and series :
              - missing dwc2 phy-names[2]
              - patchsets to add DVFS on G12a[3] which have deps on [4] and [5]

[1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/
[2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/
[3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/
[4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/
[5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/

Guillaume La Roque (6):
  dt-bindings: thermal: Add DT bindings documentation for Amlogic
    Thermal
  thermal: amlogic: Add thermal driver to support G12 SoCs
  arm64: dts: amlogic: g12: add temperature sensor
  arm64: dts: meson: sei510: Add minimal thermal zone
  arm64: dts: amlogic: odroid-n2: add minimal thermal zone
  MAINTAINERS: add entry for Amlogic Thermal driver

 .../bindings/thermal/amlogic,thermal.yaml     |  54 +++
 MAINTAINERS                                   |   9 +
 .../boot/dts/amlogic/meson-g12-common.dtsi    |  20 ++
 .../boot/dts/amlogic/meson-g12a-sei510.dts    |  56 +++
 .../boot/dts/amlogic/meson-g12b-odroid-n2.dts |  60 ++++
 drivers/thermal/Kconfig                       |  11 +
 drivers/thermal/Makefile                      |   1 +
 drivers/thermal/amlogic_thermal.c             | 336 ++++++++++++++++++
 8 files changed, 547 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
 create mode 100644 drivers/thermal/amlogic_thermal.c

-- 
2.17.1

^ permalink raw reply

* [PATCH v2 2/2] dt-bindings: net: meson-dwmac: convert to yaml
From: Neil Armstrong @ 2019-08-06 12:50 UTC (permalink / raw)
  To: robh+dt
  Cc: Neil Armstrong, martin.blumenstingl, devicetree, netdev,
	linux-amlogic, linux-arm-kernel, linux-kernel
In-Reply-To: <20190806125041.16105-1-narmstrong@baylibre.com>

Now that we have the DT validation in place, let's convert the device tree
bindings for the Synopsys DWMAC Glue for Amlogic SoCs over to a YAML schemas.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/net/amlogic,meson-dwmac.yaml     | 113 ++++++++++++++++++
 .../devicetree/bindings/net/meson-dwmac.txt   |  71 -----------
 .../devicetree/bindings/net/snps,dwmac.yaml   |   5 +
 3 files changed, 118 insertions(+), 71 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/meson-dwmac.txt

diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
new file mode 100644
index 000000000000..ae91aa9d8616
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 BayLibre, SAS
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/amlogic,meson-dwmac.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson DWMAC Ethernet controller
+
+maintainers:
+  - Neil Armstrong <narmstrong@baylibre.com>
+  - Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+
+# We need a select here so we don't match all nodes with 'snps,dwmac'
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - amlogic,meson6-dwmac
+          - amlogic,meson8b-dwmac
+          - amlogic,meson8m2-dwmac
+          - amlogic,meson-gxbb-dwmac
+          - amlogic,meson-axg-dwmac
+  required:
+    - compatible
+
+allOf:
+  - $ref: "snps,dwmac.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson8b-dwmac
+              - amlogic,meson8m2-dwmac
+              - amlogic,meson-gxbb-dwmac
+              - amlogic,meson-axg-dwmac
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: GMAC main clock
+            - description: First parent clock of the internal mux
+            - description: Second parent clock of the internal mux
+
+        clock-names:
+          minItems: 3
+          maxItems: 3
+          items:
+            - const: stmmaceth
+            - const: clkin0
+            - const: clkin1
+
+        amlogic,tx-delay-ns:
+          $ref: /schemas/types.yaml#definitions/uint32
+          description:
+            The internal RGMII TX clock delay (provided by this driver) in
+            nanoseconds. Allowed values are 0ns, 2ns, 4ns, 6ns.
+            When phy-mode is set to "rgmii" then the TX delay should be
+            explicitly configured. When not configured a fallback of 2ns is
+            used. When the phy-mode is set to either "rgmii-id" or "rgmii-txid"
+            the TX clock delay is already provided by the PHY. In that case
+            this property should be set to 0ns (which disables the TX clock
+            delay in the MAC to prevent the clock from going off because both
+            PHY and MAC are adding a delay).
+            Any configuration is ignored when the phy-mode is set to "rmii".
+
+properties:
+  compatible:
+    additionalItems: true
+    maxItems: 3
+    items:
+      - enum:
+          - amlogic,meson6-dwmac
+          - amlogic,meson8b-dwmac
+          - amlogic,meson8m2-dwmac
+          - amlogic,meson-gxbb-dwmac
+          - amlogic,meson-axg-dwmac
+    contains:
+      enum:
+        - snps,dwmac-3.70a
+        - snps,dwmac
+
+  reg:
+    items:
+      - description:
+          The first register range should be the one of the DWMAC controller
+      - description:
+          The second range is is for the Amlogic specific configuration
+          (for example the PRG_ETHERNET register range on Meson8b and newer)
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - phy-mode
+
+examples:
+  - |
+    ethmac: ethernet@c9410000 {
+         compatible = "amlogic,meson-gxbb-dwmac", "snps,dwmac";
+         reg = <0xc9410000 0x10000>, <0xc8834540 0x8>;
+         interrupts = <8>;
+         interrupt-names = "macirq";
+         clocks = <&clk_eth>, <&clkc_fclk_div2>, <&clk_mpll2>;
+         clock-names = "stmmaceth", "clkin0", "clkin1";
+         phy-mode = "rgmii";
+    };
diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
deleted file mode 100644
index 1321bb194ed9..000000000000
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ /dev/null
@@ -1,71 +0,0 @@
-* Amlogic Meson DWMAC Ethernet controller
-
-The device inherits all the properties of the dwmac/stmmac devices
-described in the file stmmac.txt in the current directory with the
-following changes.
-
-Required properties on all platforms:
-
-- compatible:	Depending on the platform this should be one of:
-			- "amlogic,meson6-dwmac"
-			- "amlogic,meson8b-dwmac"
-			- "amlogic,meson8m2-dwmac"
-			- "amlogic,meson-gxbb-dwmac"
-			- "amlogic,meson-axg-dwmac"
-		Additionally "snps,dwmac" and any applicable more
-		detailed version number described in net/stmmac.txt
-		should be used.
-
-- reg:	The first register range should be the one of the DWMAC
-	controller. The second range is is for the Amlogic specific
-	configuration (for example the PRG_ETHERNET register range
-	on Meson8b and newer)
-
-Required properties on Meson8b, Meson8m2, GXBB and newer:
-- clock-names:	Should contain the following:
-		- "stmmaceth" - see stmmac.txt
-		- "clkin0" - first parent clock of the internal mux
-		- "clkin1" - second parent clock of the internal mux
-
-Optional properties on Meson8b, Meson8m2, GXBB and newer:
-- amlogic,tx-delay-ns:	The internal RGMII TX clock delay (provided
-			by this driver) in nanoseconds. Allowed values
-			are: 0ns, 2ns, 4ns, 6ns.
-			When phy-mode is set to "rgmii" then the TX
-			delay should be explicitly configured. When
-			not configured a fallback of 2ns is used.
-			When the phy-mode is set to either "rgmii-id"
-			or "rgmii-txid" the TX clock delay is already
-			provided by the PHY. In that case this
-			property should be set to 0ns (which disables
-			the TX clock delay in the MAC to prevent the
-			clock from going off because both PHY and MAC
-			are adding a delay).
-			Any configuration is ignored when the phy-mode
-			is set to "rmii".
-
-Example for Meson6:
-
-	ethmac: ethernet@c9410000 {
-		compatible = "amlogic,meson6-dwmac", "snps,dwmac";
-		reg = <0xc9410000 0x10000
-		       0xc1108108 0x4>;
-		interrupts = <0 8 1>;
-		interrupt-names = "macirq";
-		clocks = <&clk81>;
-		clock-names = "stmmaceth";
-	}
-
-Example for GXBB:
-	ethmac: ethernet@c9410000 {
-		compatible = "amlogic,meson-gxbb-dwmac", "snps,dwmac";
-		reg = <0x0 0xc9410000 0x0 0x10000>,
-			<0x0 0xc8834540 0x0 0x8>;
-		interrupts = <0 8 1>;
-		interrupt-names = "macirq";
-		clocks = <&clkc CLKID_ETH>,
-				<&clkc CLKID_FCLK_DIV2>,
-				<&clkc CLKID_MPLL2>;
-		clock-names = "stmmaceth", "clkin0", "clkin1";
-		phy-mode = "rgmii";
-	};
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 4377f511a51d..c78be15704b9 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -50,6 +50,11 @@ properties:
         - allwinner,sun8i-r40-emac
         - allwinner,sun8i-v3s-emac
         - allwinner,sun50i-a64-emac
+        - amlogic,meson6-dwmac
+        - amlogic,meson8b-dwmac
+        - amlogic,meson8m2-dwmac
+        - amlogic,meson-gxbb-dwmac
+        - amlogic,meson-axg-dwmac
         - snps,dwmac
         - snps,dwmac-3.50a
         - snps,dwmac-3.610
-- 
2.22.0

^ permalink raw reply related

* [PATCH v2 1/2] dt-bindings: net: snps,dwmac: update reg minItems maxItems
From: Neil Armstrong @ 2019-08-06 12:50 UTC (permalink / raw)
  To: robh+dt
  Cc: Neil Armstrong, martin.blumenstingl, devicetree, netdev,
	linux-amlogic, linux-arm-kernel, linux-kernel
In-Reply-To: <20190806125041.16105-1-narmstrong@baylibre.com>

The Amlogic Meson DWMAC glue bindings needs a second reg cells for the
glue registers, thus update the reg minItems/maxItems to allow more
than a single reg cell.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 76fea2be66ac..4377f511a51d 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -61,7 +61,8 @@ properties:
         - snps,dwxgmac-2.10
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   interrupts:
     minItems: 1
-- 
2.22.0

^ permalink raw reply related

* [PATCH v2 0/2] dt-bindings: net: meson-dwmac: convert to yaml
From: Neil Armstrong @ 2019-08-06 12:50 UTC (permalink / raw)
  To: robh+dt
  Cc: Neil Armstrong, martin.blumenstingl, devicetree, netdev,
	linux-amlogic, linux-arm-kernel, linux-kernel

This patchsets converts the Amlogic Meson DWMAC glue bindings over to
YAML schemas using the already converted dwmac bindings.

The first patch is needed because the Amlogic glue needs a supplementary
reg cell to access the DWMAC glue registers.

Neil Armstrong (2):
  dt-bindings: net: snps,dwmac: update reg minItems maxItems
  dt-bindings: net: meson-dwmac: convert to yaml

 .../bindings/net/amlogic,meson-dwmac.yaml     | 113 ++++++++++++++++++
 .../devicetree/bindings/net/meson-dwmac.txt   |  71 -----------
 .../devicetree/bindings/net/snps,dwmac.yaml   |   8 +-
 3 files changed, 120 insertions(+), 72 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/meson-dwmac.txt

-- 
2.22.0

^ permalink raw reply

* Re: [PATCH v2] gpio: mpc8xxx: Add new platforms GPIO DT node description
From: Linus Walleij @ 2019-08-06 12:48 UTC (permalink / raw)
  To: Hui Song
  Cc: Shawn Guo, Li Yang, Rob Herring, Mark Rutland,
	Bartosz Golaszewski, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM
In-Reply-To: <20190806024923.34355-1-hui.song_1@nxp.com>

Hi Hui,

On Tue, Aug 6, 2019 at 4:59 AM Hui Song <hui.song_1@nxp.com> wrote:

> From: Song Hui <hui.song_1@nxp.com>
>
> Update the NXP GPIO node dt-binding file for QorIQ and
> Layerscape platforms, and add one more example with
> ls1028a GPIO node.
>
> Signed-off-by: Song Hui <hui.song_1@nxp.com>
(...)
> +Example of gpio-controller node for a ls1028a SoC:
> +
> +gpio1: gpio@2300000 {
> +       compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";

What you need to do is to add "fsl,ls1028a-gpio" to the list
of compatible values at the top of the file "Required properties".
Please send a v2 with this fixed.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH] dt-bindings: net: meson-dwmac: convert to yaml
From: Neil Armstrong @ 2019-08-06 12:46 UTC (permalink / raw)
  To: Rob Herring
  Cc: Martin Blumenstingl, devicetree, netdev, linux-amlogic,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+efvvb1UK-Nas0G5XefLWwN7ebnqoevi+W=jj4r3E2dg@mail.gmail.com>

On 06/08/2019 00:09, Rob Herring wrote:
> On Mon, Aug 5, 2019 at 6:26 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> Now that we have the DT validation in place, let's convert the device tree
>> bindings for the Synopsys DWMAC Glue for Amlogic SoCs over to a YAML schemas.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> Rob,
>>
>> I keep getting :
>> .../devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: reg: [[3376480256, 65536], [3364046144, 8]] is too long
> 
> Because snps,dwmac.yaml has:
> 
>   reg:
>     maxItems: 1
> 
> The schemas are applied separately and all have to be valid. You'll
> need to change snps,dwmac.yaml to:
> 
> reg:
>   minItems: 1
>   maxItems: 2
> 
> 
> The schema error messages leave something to be desired. I wish the
> error messages said which schema is throwing the error.

Indeed, it fixed it

> 
>> for the example DT
>>
>> and for the board DT :
>> ../amlogic/meson-gxl-s905x-libretech-cc.dt.yaml: ethernet@c9410000: reg: [[0, 3376480256, 0, 65536, 0, 3364046144, 0, 4]] is too short
>> ../amlogic/meson-gxl-s905x-nexbox-a95x.dt.yaml: soc: ethernet@c9410000:reg:0: [0, 3376480256, 0, 65536, 0, 3364046144, 0, 4] is too long
>>
>> and I don't know how to get rid of it.
> 
> The first issue is the same as the above. The 2nd issue is the use of
> <> in dts files becomes stricter with the schema. Each entry in an
> array needs to be bracketed:
> 
> reg = <0x0 0xc9410000 0x0 0x10000>,
>           <0x0 0xc8834540 0x0 0x4>;

I did it but somehow it was overrided (with the same content) in another .dtsi
included file...

Sorry for the noise !

Neil

> 
> Rob
> 

^ 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