Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] ARM64: configs: Add Platform MHU in defconfig
From: Neil Armstrong @ 2016-11-03  9:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478164284-24958-1-git-send-email-narmstrong@baylibre.com>

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index dab2cb0..6631bda 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -394,6 +394,7 @@ CONFIG_MSM_MMCC_8996=y
 CONFIG_HWSPINLOCK_QCOM=y
 CONFIG_MAILBOX=y
 CONFIG_ARM_MHU=y
+CONFIG_PLATFORM_MHU=y
 CONFIG_HI6220_MBOX=y
 CONFIG_ARM_SMMU=y
 CONFIG_QCOM_SMEM=y
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/2] ARM64: dts: meson-gxbb: Add generic legacy scpi compatible
From: Neil Armstrong @ 2016-11-03  9:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478164284-24958-1-git-send-email-narmstrong@baylibre.com>

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 2d69a3b..5394657 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -75,7 +75,7 @@
 	};
 
 	scpi {
-		compatible = "amlogic,meson-gxbb-scpi";
+		compatible = "amlogic,meson-gxbb-scpi", "arm,legacy-scpi";
 		mboxes = <&mailbox 1 &mailbox 2>;
 		shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/2] ARM64: meson-gxbb: SCPI Fixup
From: Neil Armstrong @ 2016-11-03  9:11 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset updates the GXBB dtsi and the ARM64 defconfig in order
to make SCPI work following the Legacy SCPI rework from Sudeep Hola at [1].

The rework introduced a generic "arm,legacy-scpi" compatible string.
The second patch enables the necessary Platform MHU in defconfig.

[1] http://lkml.kernel.org/r/1478148731-11712-1-git-send-email-sudeep.holla at arm.com

Neil Armstrong (2):
  ARM64: dts: meson-gxbb: Add generic legacy scpi compatible
  ARM64: configs: Add Platform MHU in defconfig

 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 2 +-
 arch/arm64/configs/defconfig                | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH 0/5] drm/sun4i: Handle TV overscan
From: Maxime Ripard @ 2016-11-03  9:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161031084233.GS1041@n2100.armlinux.org.uk>

Hi Russell,

On Mon, Oct 31, 2016 at 08:42:34AM +0000, Russell King - ARM Linux wrote:
> On Tue, Oct 18, 2016 at 12:03:49PM +0200, Maxime Ripard wrote:
> > The first one is that this overscanning should be reported by the
> > connector I guess? but this is really TV specific, so we need one way
> > to let the user tell how the image is displayed on its side, and we
> > cannot really autodetect it, and this needs to be done at runtime so
> > that we can present some shiny interface to let it select which
> > overscan ratio works for him/her.
> 
> See xbmc... they go through a nice shiny setup which includes adjusting
> the visible area.  From what I remember, it has pointers on each corner
> which you can adjust to be just visible on the screen, so xbmc knows
> how much overscan there is, and xbmc itself reduces down to the user
> set size.

Yes. And that is an XBMC only solution, that doesn't work with the
fbdev emulation and is probably doing an additional composition to
scale down and center their frames through OpenGL.

We might not have a GPU in the system, and we might not even have an
entire graphic stack on top either, so I don't think fixing at the
user-space level is a good option (especially since we already have an
overscan property in DRM).

> > The second one is that we still need to expose the reduced modes to
> > userspace, and not only the displayed size, so that the applications
> > know what they must draw on. But I guess this could be adjusted by the
> > core too.
> > 
> > In order to work consistently, I think all planes should be adjusted
> > that way, so that relative coordinates are from the primary plane
> > origin, and not the display origin. But that could be adjusted too by
> > the core I guess.
> 
> I'm not sure about that - we want the graphics to be visible, but that
> may not be appropriate for an video overlay frame.  It's quite common
> for (eg) broadcast video to contain dead pixels or other artifacts on
> the right hand side, and the broadcast video expects overscan to be
> present.
>
> I know this because I have run my TV with overscan disabled, even for
> broadcast TV.

I know, but on this particular hardware, composite really is just
another video output. There's not even a TV receiver in it, so I don't
think we have to worry about it.

> > The fourth one being the major one. Every time I raised the issue on
> > IRC, the answer basically was "we don't care about analog", so I'm a
> > bit pessimistic about whether dealing with this in the core would be
> > accepted, hence why I chose to deal with this at the driver level.
> 
> Yea, that's quite sad, "analog" has become a dirty word, but really
> this has nothing to do with "analog" at all - there are LCD TVs (and
> some monitors) out there which take HDMI signals but refuse to
> disable overscan no matter what you do to them if you provide them
> with a "broadcast"  mode - so the analog excuse is very poor.

I'd agree with you, but I was also told to not turn that into a
generic code and deal with that in my driver.

Do you have any suggestions?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/29a096c4/attachment.sig>

^ permalink raw reply

* [PATCH v2 0/6] STM32F4 Add RTC & QSPI clocks
From: Gabriel Fernandez @ 2016-11-03  8:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <68855ca8-a74f-7c76-342f-de0aeec51ca1@st.com>

Hi Alexandre,


On 11/03/2016 09:52 AM, Alexandre Torgue wrote:
> Hi Gabriel,
>
> On 10/14/2016 11:18 AM, gabriel.fernandez at st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> v2:
>>  - rename compatible property "st,stm32f46xx-rcc" into 
>> "st,stm32f469-rcc"
>>  - cosmetic: remove bad copy/paste
>>
>> This patch-set introduce RTC and QSPI clocks for STM32F4 socs
>> RTC clock has 3 parents clock oscillators (lsi/lse/hse_rtc)
>>
>> example to use rtc clock:
>>
>>         rtc: rtc at 40002800 {
>>             compatible = "st,stm32-rtc";
>>             reg = <0x40002800 0x400>;
>>             ...
>>             clocks = <&rcc 1 CLK_RTC>;
>>
>>             assigned-clocks =  <&rcc 1 CLK_RTC>;
>>             assigned-clock-parents = <&rcc 1 CLK_LSE>;
>>             ...
>>         };
>>
>> Gabriel Fernandez (6):
>>   clk: stm32f4: Add LSI & LSE clocks
>>   ARM: dts: stm32f429: add LSI and LSE clocks
>>   arm: stmf32: Enable SYSCON
>>   clk: stm32f4: Add RTC clock
>>   clk: stm32f469: Add QSPI clock
>>   ARM: dts: stm32f429: Add QSPI clock
>
> You sent a V3 without DT patches. Should I take DT patches from this 
> V2 patchset ?
>
> Regards
> Alex
>
Yes, no problem

Thanks !

BR.

Gabriel
>
>>
>>  .../devicetree/bindings/clock/st,stm32-rcc.txt     |   4 +-
>>  arch/arm/boot/dts/stm32f429.dtsi                   |  18 +
>>  arch/arm/boot/dts/stm32f469-disco.dts              |   4 +
>>  arch/arm/configs/stm32_defconfig                   |   1 +
>>  drivers/clk/clk-stm32f4.c                          | 442 
>> ++++++++++++++++++++-
>>  5 files changed, 447 insertions(+), 22 deletions(-)
>>

^ permalink raw reply

* [PATCH] clk: rockchip: fix some clocks' name to be more standard style
From: Shawn Lin @ 2016-11-03  8:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478070296-23080-1-git-send-email-jay.xu@rock-chips.com>

On 2016/11/2 15:04, Jianqun Xu wrote:
> Fix aclk_emmcgrf to aclk_emmc_grf, and fix aclk_emmccore to be
> aclk_emmc_core.
>

What is the standard style should be?

TRM uses aclk_emmccore but not aclk_emmc_core, so should it be more
standrad to keep it as-is?

> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/clk/rockchip/clk-rk3399.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 2c7cba7..b3df2c6 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -935,11 +935,11 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
>  			RK3399_CLKGATE_CON(6), 13, GFLAGS),
>  	COMPOSITE_NOGATE(ACLK_EMMC, "aclk_emmc", mux_aclk_emmc_p, CLK_IGNORE_UNUSED,
>  			RK3399_CLKSEL_CON(21), 7, 1, MFLAGS, 0, 5, DFLAGS),
> -	GATE(ACLK_EMMC_CORE, "aclk_emmccore", "aclk_emmc", CLK_IGNORE_UNUSED,
> +	GATE(ACLK_EMMC_CORE, "aclk_emmc_core", "aclk_emmc", CLK_IGNORE_UNUSED,
>  			RK3399_CLKGATE_CON(32), 8, GFLAGS),
>  	GATE(ACLK_EMMC_NOC, "aclk_emmc_noc", "aclk_emmc", CLK_IGNORE_UNUSED,
>  			RK3399_CLKGATE_CON(32), 9, GFLAGS),
> -	GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", CLK_IGNORE_UNUSED,
> +	GATE(ACLK_EMMC_GRF, "aclk_emmc_grf", "aclk_emmc", CLK_IGNORE_UNUSED,
>  			RK3399_CLKGATE_CON(32), 10, GFLAGS),
>
>  	/* perilp0 */
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply

* [PATCH v2 0/6] STM32F4 Add RTC & QSPI clocks
From: Alexandre Torgue @ 2016-11-03  8:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476436699-21879-1-git-send-email-gabriel.fernandez@st.com>

Hi Gabriel,

On 10/14/2016 11:18 AM, gabriel.fernandez at st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> v2:
>  - rename compatible property "st,stm32f46xx-rcc" into "st,stm32f469-rcc"
>  - cosmetic: remove bad copy/paste
>
> This patch-set introduce RTC and QSPI clocks for STM32F4 socs
> RTC clock has 3 parents clock oscillators (lsi/lse/hse_rtc)
>
> example to use rtc clock:
>
> 		rtc: rtc at 40002800 {
> 			compatible = "st,stm32-rtc";
> 			reg = <0x40002800 0x400>;
> 			...
> 			clocks = <&rcc 1 CLK_RTC>;
>
> 			assigned-clocks =  <&rcc 1 CLK_RTC>;
> 			assigned-clock-parents = <&rcc 1 CLK_LSE>;
> 			...
> 		};
>
> Gabriel Fernandez (6):
>   clk: stm32f4: Add LSI & LSE clocks
>   ARM: dts: stm32f429: add LSI and LSE clocks
>   arm: stmf32: Enable SYSCON
>   clk: stm32f4: Add RTC clock
>   clk: stm32f469: Add QSPI clock
>   ARM: dts: stm32f429: Add QSPI clock

You sent a V3 without DT patches. Should I take DT patches from this V2 
patchset ?

Regards
Alex


>
>  .../devicetree/bindings/clock/st,stm32-rcc.txt     |   4 +-
>  arch/arm/boot/dts/stm32f429.dtsi                   |  18 +
>  arch/arm/boot/dts/stm32f469-disco.dts              |   4 +
>  arch/arm/configs/stm32_defconfig                   |   1 +
>  drivers/clk/clk-stm32f4.c                          | 442 ++++++++++++++++++++-
>  5 files changed, 447 insertions(+), 22 deletions(-)
>

^ permalink raw reply

* [PATCH v4 6/8] dt-bindings: Add support for Amlogic GXBB SCPI Interface
From: Neil Armstrong @ 2016-11-03  8:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161102222050.GA32189@e106835-lin.cambridge.arm.com>

On 11/02/2016 11:20 PM, Sudeep Holla wrote:
> On Sat, Oct 29, 2016 at 11:39:05AM -0700, Olof Johansson wrote:
> 
> I will rework the patches to address the concerns as I too did share same
> concern.
> 
> 
> Hi Neil,
> 
> You may need to rework the DTS files based on that, please be aware of
> that and make the necessary changes.


Hi,

I will post the necessary changes since kevin already applied the previous patches.

Thanks,
Neil

> 
> --
> Regards,
> Sudeep
> 

^ permalink raw reply

* [PATCH v3] ARM: dts: Add gyro and accel to APQ8060 Dragonboard
From: Linus Walleij @ 2016-11-03  8:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161101231708.GM25787@tuxbot>

[CC Quan who made a nice hierarchical domain]

On Wed, Nov 2, 2016 at 12:17 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Tue 01 Nov 04:31 PDT 2016, Linus Walleij wrote:
>
>> On Mon, Oct 31, 2016 at 11:20 PM, Bjorn Andersson
>> <bjorn.andersson@linaro.org> wrote:
>>
>> >> +                                     interrupt-parent = <&pm8058_gpio>;
>> >> +                                     interrupts = <208 IRQ_TYPE_EDGE_FALLING>;
>> >
>> > To remove the need of resetting the interrupt-parent in each child you
>> > can use the following form:
>> >
>> >     interrupts-extended = <&pm8058_gpio 208 IRQ_TYPE_EDGE_FALLING>;
>> >
>> > But, if we correct the ssbi gpio driver then this would no longer be
>> > interrupt 208 in this parent, right?. I believe that if you say
>> > <&pmicintc 208 IRQ_TYPE_EDGE_FALLING> instead, which should work even
>> > after we correct the gpio translation.
>>
>> Yes. It should be fixed everywhere but is not related to this
>> patch. But I can do a two-patch series first fixing this and then
>> adding the gyro+accelerometer on top referencing the MFD
>> pmicintc as parent.
>>
>
> Sorry, I'm not sure I'm following.
>
> What I tried to say was that before the correction of the GPIO block's
> window this particular IRQ would be #208 in &pmicintc and #208 in
> &pm8058_gpio. After the correction of the window the IRQ is #208 in
> &pmicintc and #17 in &pm8058_gpio.
>
> As such, using #208 from &pmicintc would make this work through the
> correction of the GPIO driver. Perhaps I'm wrong about this?

No that is exactly what I mean. I just mean I should not use
the pm8058_gpio as interrupt parent at all for now.

(You will see in my patch set, cooking it.)

>> What needs to happen is for the SSBI and SPMI GPIO to use a
>> hierarchical irqdomain so their GPIO local line offset and hwirq
>> are the same. Then we can reference the GPIO IRQ lines directly
>> in a correct manner.
>
> Right. Do you have any example of drivers getting this right? I did take
> a quick look but didn't find one.

It seems drivers/gpio/gpio-xgene-sb.c is doing something like
what we want. See for example how it picks out the parent domain
and hangs a new domain off it.

If possible, I would like to add infrastructure for this to gpiolib,
if it can be made generic and make sense.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v2 13/14] ARM: dts: sun6i: hummingbird: Enable internal audio codec
From: Maxime Ripard @ 2016-11-03  8:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-14-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:55PM +0800, Chen-Yu Tsai wrote:
> The Hummingbird A31 has headset and line in audio jacks and an onboard
> mic routed to the pins for the SoC's internal codec. The line out pins
> are routed to an onboard speaker amp, whose output is available on a
> pin header.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> index 9a74637f677f..48c041b75aab 100644
> --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> @@ -69,6 +69,19 @@
>  	};
>  };
>  
> +&codec {
> +	allwinner,audio-routing =
> +		"Headphone", "HP",
> +		"Speaker", "LINEOUT",
> +		"LINEIN", "Line In",
> +		"MIC1", "Mic",
> +		"MIC2", "Headset Mic",
> +		"Mic",	"MBIAS",
> +		"Headset Mic", "HBIAS";
> +	allwinner,pa-gpios = <&pio 7 22 GPIO_ACTIVE_HIGH>; /* PH22 */
> +	status = "okay";
> +};
> +
>  &cpu0 {
>  	cpu-supply = <&reg_dcdc3>;
>  };
> @@ -152,6 +165,13 @@
>  };
>  
>  &pio {
> +	codec_pa_pin: codec_pa_pin at 0 {
> +		allwinner,pins = "PH22";
> +		allwinner,function = "gpio_out";
> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> +	};
> +

This pin group isn't used anywhere. Because of the strict thing in
pinctrl, I'd say it's better to not set it, but then, the pin group is
useless.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/81e008e2/attachment.sig>

^ permalink raw reply

* [linux-sunxi] [PATCH v2 04/14] ASoC: sun4i-codec: Increase DMA max burst to 8
From: Priit Laes @ 2016-11-03  8:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-5-wens@csie.org>

On Thu, 2016-11-03 at 15:55 +0800, Chen-Yu Tsai wrote:
> According to the DMA engine API documentation, maxburst denotes the
> largest possible size of a single transfer, so as not to overflow
> destination FIFOs as explained in this excerpt from dmaengine.h
> 
> ?* @src_maxburst: the maximum number of words (note: words, as in
> ?* units of the src_addr_width member, not bytes) that can be sent
> ?* in one burst to the device. Typically something like half the
> ?* FIFO depth on I/O peripherals so you don't overflow it. This
> ?* may or may not be applicable on memory sources.
> ?* @dst_maxburst: same as src_maxburst but for destination target

> ?* mutatis mutandis.

^^ :)

> 
> The TX FIFO is 64 samples deep for stereo, and the RX FIFO is 16
> samples deep. So maxburst could be 32 and 8 for TX and RX
> respectively.
> 
> Unfortunately the sunxi DMA controller driver takes maxburst as
> the requested burst size, rather than a limit, and returns an error
> for unsupported values. The original value was 4, but some later
> SoCs do not officially support this burst size.
> 
> This patch increases maxburst on the TX side to 8, which is supported
> by all variants of the sunxi DMA controller.
> 
> Cc: Vinod Koul <vinod.koul@intel.com>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> ?sound/soc/sunxi/sun4i-codec.c | 4 ++--
> ?1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-
> codec.c
> index 61ae502a5061..d867b96d367b 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -886,12 +886,12 @@ static int sun4i_codec_probe(struct
> platform_device *pdev)
> ?
> ?	/* DMA configuration for TX FIFO */
> ?	scodec->playback_dma_data.addr = res->start + quirks-
> >reg_dac_txdata;
> -	scodec->playback_dma_data.maxburst = 4;
> +	scodec->playback_dma_data.maxburst = 8;
> ?	scodec->playback_dma_data.addr_width =
> DMA_SLAVE_BUSWIDTH_2_BYTES;
> ?
> ?	/* DMA configuration for RX FIFO */
> ?	scodec->capture_dma_data.addr = res->start + quirks-
> >reg_adc_rxdata;
> -	scodec->capture_dma_data.maxburst = 4;
> +	scodec->capture_dma_data.maxburst = 8;
> ?	scodec->capture_dma_data.addr_width =
> DMA_SLAVE_BUSWIDTH_2_BYTES;
> ?
> ?	ret = snd_soc_register_codec(&pdev->dev, quirks->codec,
> --?
> 2.10.2
> 

^ permalink raw reply

* [PATCH 09/13] mmc: dw_mmc: remove the "clock-freq-min-max" property
From: Jaehoon Chung @ 2016-11-03  8:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20823767.hqlyMd1aKN@diego>

Hi Heiko.

On 11/03/2016 05:20 PM, Heiko St?bner wrote:
> Hi Jaehoon,
> 
> Am Donnerstag, 3. November 2016, 15:21:31 schrieb Jaehoon Chung:
>> Remove the "clock-freq-min-max" property.
>> There is "max-frequency" property in drivers/mmc/core/host.c
>> It can be used for getting maximum frequency.
>>
>> And minimum clock value is assigned to 100K by default.
>> Because MMC core should check the initial clock value from
>> 400K to 100K until finding correct value.
>> It's why Minimum value puts 100K by default.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt |  3 ---
>>  drivers/mmc/host/dw_mmc.c                                  | 13
>> ++++--------- 2 files changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt index
>> 4e00e85..21c8251 100644
>> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> @@ -56,9 +56,6 @@ Optional properties:
>>    is specified and the ciu clock is specified then we'll try to set the ciu
>> clock to this at probe time.
>>
>> -* clock-freq-min-max: Minimum and Maximum clock frequency for card output
>> -  clock(cclk_out). If it's not specified, max is 200MHZ and min is 400KHz
>> by default. -
> 
> I'd think devicetree people will protest about simply removing this property 
> without deprecating it first - to be backwards compatible with old devicetrees.
> 
> Especially as this property was added already back in 2013, so has been in use 
> for quite some time.

Thanks for pointing out. I will resend the patchset with changing this.
And maintain the backwards compatible.

Best Regards,
Jaehoon Chung

> 
> 
> Heiko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

^ permalink raw reply

* [PATCH] ARM: DT: stm32: move dma translation to board files
From: Alexandre Torgue @ 2016-11-03  8:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAF3+TqcAjzAJNfikRKfccmYX778FuxNSeVbT1dGU0hswxZWjjg@mail.gmail.com>

Hi Bruno,

On 11/02/2016 07:23 PM, Bruno Herrera wrote:
> On Wed, Nov 2, 2016 at 4:05 PM, Rados?aw Pietrzyk
> <radoslaw.pietrzyk@gmail.com> wrote:
>> Have you tried with
>>
>> sdio {
>> ...
>> compatible = "arm,primecell";
>> arm,primecell-periphid = <id_to_define>;
>> ...
>> }
>>
>> This should register proper amba device and give it an ID you wish.
>
> I'm using this:
>
> sdio: sdio at 40012c00 {
> compatible = "arm,pl18x", "arm,primecell";
> arm,primecell-periphid = <0x00480181>;
> reg = <0x40012c00 0x400>;
> dmas =  <&dma2 6 4 0x10400 0x3>, /* Logical - DevToMem */
> <&dma2 3 4 0x10400 0x3>; /* Logical - MemToDev */
> dma-names = "rx", "tx";
> clocks = <&rcc 0 171>;
> clock-names = "apb_pclk";
> interrupts = <49>;
> status = "disabled";
> };
>
> And on the driver side I added this:
>
> /* STM32 variants */
>
> {
>
>         .id     = 0x00480181,
>
>         .mask   = 0xf0ffffff,
>
>         .data   = &variant_stm32f4x9,
>
> },
>
> My .id field is the .id from variant_ux500 plus one (and what I
> believe is not valid).
> And I cannot read the ID from STM32 IP because it always return zero.
> Maybe someone from ST(@Alex) can get the real ID. ;)

I will have a look on it soon.

regards
alex


>
>
>>
>> 2016-11-02 17:07 GMT+01:00 Bruno Herrera <bruherrera@gmail.com>:
>>>
>>> Hi
>>>
>>> On Wed, Nov 2, 2016 at 12:32 PM, Alexandre Torgue
>>> <alexandre.torgue@st.com> wrote:
>>>> Hi
>>>>
>>>> On 10/31/2016 07:58 PM, Rados?aw Pietrzyk wrote:
>>>>>
>>>>> I think wlcore driver searches dma-ranges in its parent that's why sdio
>>>>> node needs it.
>>>>
>>>>
>>>> Yes I agree. In this case it is needed as you have subnode in sdio node.
>>>> So IMO empty dma-ranges could be removed from ethernet and usb node, but
>>>> kept in future sdio subnode.
>>>
>>> Now it is clear.
>>>
>>>>
>>>> Bruno,
>>>> Do you plan to push sdio support ?
>>>
>>> Yes I do, but I'm not sure how long it will take. The I had to
>>> change(and hack) the mmci code because I could not get the ID from
>>> STM32 SDIO IP.
>>> My current WIP is at @
>>>
>>> https://github.com/mcoquelin-stm32/afboot-stm32/pull/4#issuecomment-247571615
>>> I know Andrea Merello is also working on that (and he probably has a
>>> more complete patch).
>>>
>>>>
>>>>
>>>>
>>>>>
>>>>> 2016-10-31 17:41 GMT+01:00 Bruno Herrera <bruherrera@gmail.com
>>>>> <mailto:bruherrera@gmail.com>>:
>>>>>
>>>>>     On Mon, Oct 31, 2016 at 12:14 PM, Rados?aw Pietrzyk
>>>>>     <radoslaw.pietrzyk at gmail.com <mailto:radoslaw.pietrzyk@gmail.com>>
>>>>>     wrote:
>>>>>     > This is weird because dma ddresses are recalculated using
>>>>> parent's
>>>>>     > dma-ranges property and soc already has it so there should be
>>>>> absolutely no
>>>>>     > problem.
>>>>>
>>>>>     These are my DTS and DTSI file.
>>>>>     >
>>>>>     > 2016-10-31 11:27 GMT+01:00 Bruno Herrera <bruherrera@gmail.com
>>>>>     <mailto:bruherrera@gmail.com>>:
>>>>>     >>
>>>>>     >> On Fri, Oct 28, 2016 at 5:09 AM, Rados?aw Pietrzyk
>>>>>     >> <radoslaw.pietrzyk@gmail.com
>>>>>     <mailto:radoslaw.pietrzyk@gmail.com>> wrote:
>>>>>     >> > Have you defined your sdio node within soc node ?
>>>>>     >>
>>>>>     >> It is in the SOC node of the DSTI file.
>>>>>     >>
>>>>>     >> >
>>>>>     >> > 2016-10-27 14:57 GMT+02:00 Bruno Herrera <bruherrera@gmail.com
>>>>>     <mailto:bruherrera@gmail.com>>:
>>>>>     >> >>
>>>>>     >> >> Hi Alex,
>>>>>     >> >>
>>>>>     >> >> On Thu, Oct 27, 2016 at 10:21 AM, Alexandre Torgue
>>>>>     >> >> <alexandre.torgue at st.com <mailto:alexandre.torgue@st.com>>
>>>>> wrote:
>>>>>     >> >> > Hi Bruno,
>>>>>     >> >> >
>>>>>     >> >> >
>>>>>     >> >> > On 10/27/2016 12:43 PM, Bruno Herrera wrote:
>>>>>     >> >> >>
>>>>>     >> >> >> Hi Alex,
>>>>>     >> >> >>
>>>>>     >> >> >> On Wed, Oct 26, 2016 at 7:09 AM, Alexandre Torgue
>>>>>     >> >> >> <alexandre.torgue at st.com <mailto:alexandre.torgue@st.com>>
>>>>>     wrote:
>>>>>     >> >> >>>
>>>>>     >> >> >>> Hi Bruno,
>>>>>     >> >> >>>
>>>>>     >> >> >>> On 10/25/2016 11:06 PM, Bruno Herrera wrote:
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>> Hi Alexandre,
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>>
>>>>>     >> >> >>>>> stm32f469-disco and stm32f429-eval boards use SDRAM
>>>>>     start address
>>>>>     >> >> >>>>> remapping
>>>>>     >> >> >>>>> (to @0) to boost performances. A DMA translation
>>>>> through
>>>>>     >> >> >>>>> "dma-ranges"
>>>>>     >> >> >>>>> property was needed for other masters than the M4 CPU.
>>>>>     >> >> >>>>> stm32f429-disco doesn't use remapping so doesn't need
>>>>>     this DMA
>>>>>     >> >> >>>>> translation.
>>>>>     >> >> >>>>> This patches moves this DMA translation definition from
>>>>>     stm32f429
>>>>>     >> >> >>>>> soc
>>>>>     >> >> >>>>> file
>>>>>     >> >> >>>>> to board files.
>>>>>     >> >> >>>>>
>>>>>     >> >> >>>>> Signed-off-by: Alexandre TORGUE
>>>>> <alexandre.torgue@st.com
>>>>>     <mailto:alexandre.torgue@st.com>>
>>>>>
>>>>>     >> >> >>>>>
>>>>>     >> >> >>>>> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>>> b/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>>> index 13c7cd2..a763c15 100644
>>>>>     >> >> >>>>> --- a/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>>> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>>> @@ -82,6 +82,10 @@
>>>>>     >> >> >>>>>                 };
>>>>>     >> >> >>>>>         };
>>>>>     >> >> >>>>>
>>>>>     >> >> >>>>> +       soc {
>>>>>     >> >> >>>>> +               dma-ranges = <0xc0000000 0x0
>>>>> 0x10000000>;
>>>>>     >> >> >>>>> +       };
>>>>>     >> >> >>>>> +
>>>>>     >> >> >>>>>         usbotg_hs_phy: usbphy {
>>>>>     >> >> >>>>>                 #phy-cells = <0>;
>>>>>     >> >> >>>>>                 compatible = "usb-nop-xceiv";
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>> Shouldn't also the peripheral dma-ranges property move
>>>>> to
>>>>>     board
>>>>>     >> >> >>>> specific
>>>>>     >> >> >>>> too?
>>>>>     >> >> >>>> I  had this patch for while but I didn't had the time to
>>>>>     submit:
>>>>>     >> >> >>>
>>>>>     >> >> >>>
>>>>>     >> >> >>>
>>>>>     >> >> >>> Well spot I forgot it. Actually, discussing with Arnd
>>>>>     ysterday on
>>>>>     >> >> >>> IIRC,
>>>>>     >> >> >>> empty dma-ranges is not needed. Can you test on your side
>>>>> by
>>>>>     >> >> >>> removing
>>>>>     >> >> >>> dma-ranges in usb node please ?
>>>>>     >> >> >>
>>>>>     >> >> >> Unfortunately will take a time for me to set up this
>>>>>     environment on
>>>>>     >> >> >> the STM32F4-EVAL board.
>>>>>     >> >> >> And on the discovery boards we dont have this scenario.
>>>>>     That was the
>>>>>     >> >> >> main reason I did not submit the patch right away.
>>>>>     >> >> >> My conclusion and I might be wrong but is based on the my
>>>>>     tests with
>>>>>     >> >> >> SDIO device at STM32F469I-DISCO board.
>>>>>     >> >> >>
>>>>>     >> >> >> I started this issue as discussion at ST Forum but Maxime
>>>>>     gave me
>>>>>     >> >> >> the
>>>>>     >> >> >> hint.
>>>>>     >> >> >>
>>>>>     >> >> >>
>>>>>     >> >> >>
>>>>>     >> >> >>
>>>>>     >> >> >>
>>>>>
>>>>>
>>>>> https://my.st.com/public/STe2ecommunities/mcu/Lists/cortex_mx_stm32/Flat.aspx?RootFolder=https%3a%2f%2fmy%2est%2ecom%2fpublic%2fSTe2ecommunities%2fmcu%2fLists%2fcortex_mx_stm32%2fDMA2%20and%20SYSCFG_MEMRMP%20relationship&FolderCTID=0x01200200770978C69A1141439FE559EB459D7580009C4E14902C3CDE46A77F0FFD06506F5B&currentviews=44
>>>>>
>>>>>
>>>>> <https://my.st.com/public/STe2ecommunities/mcu/Lists/cortex_mx_stm32/Flat.aspx?RootFolder=https%3a%2f%2fmy%2est%2ecom%2fpublic%2fSTe2ecommunities%2fmcu%2fLists%2fcortex_mx_stm32%2fDMA2%20and%20SYSCFG_MEMRMP%20relationship&FolderCTID=0x01200200770978C69A1141439FE559EB459D7580009C4E14902C3CDE46A77F0FFD06506F5B&currentviews=44>
>>>>>     >> >> >>
>>>>>     >> >> >>> I will push a v2 by removing empty dma-ranges if tests
>>>>> are
>>>>>     ok in
>>>>>     >> >> >>> your
>>>>>     >> >> >>> side.
>>>>>     >> >> >>
>>>>>     >> >> >>
>>>>>     >> >> >> From my understating/conclusion is: when empty
>>>>>     property(dma-ranges)
>>>>>     >> >> >> is
>>>>>     >> >> >> the device node, the mapping will be taken in
>>>>> consideration
>>>>>     when
>>>>>     >> >> >> using
>>>>>     >> >> >> DMA otherwise the mapping is ignored.
>>>>>     >> >> >> And in the SDIO case it is needed for DEV->MEM(SDRAM) and
>>>>>     >> >> >> MEM(SDRAM)->DEV. If it is not the case for the devices in
>>>>>     question
>>>>>     >> >> >> so
>>>>>     >> >> >> I suppose it can work without the property.
>>>>>     >> >> >
>>>>>     >> >> >
>>>>>     >> >> > For sure translation has to be done but I'm not sure that
>>>>> an
>>>>>     empty
>>>>>     >> >> > "dma-ranges" is needed in device node to activate it. For
>>>>>     Ethernet
>>>>>     >> >> > empty
>>>>>     >> >> > "dma-ranges" is not needed. I will try with usb.
>>>>>     >> >>
>>>>>     >> >> In the case of SDIO it is needed. As example this is my
>>>>>     working SDIO
>>>>>     >> >> node:
>>>>>     >> >>
>>>>>     >> >> sdio: sdio at 40012c00 {
>>>>>     >> >> compatible = "arm,pl18x", "arm,primecell";
>>>>>     >> >> arm,primecell-periphid = <0x00480181>;
>>>>>     >> >> reg = <0x40012c00 0x400>;
>>>>>     >> >> dmas =  <&dma2 6 4 0x10400 0x3>, /* Logical - DevToMem */
>>>>>     >> >> <&dma2 3 4 0x10400 0x3>; /* Logical - MemToDev */
>>>>>     >> >> dma-names = "rx", "tx";
>>>>>     >> >> clocks = <&rcc 0 171>;
>>>>>     >> >> clock-names = "apb_pclk";
>>>>>     >> >> interrupts = <49>;
>>>>>     >> >> status = "disabled";
>>>>>     >> >> };
>>>>>     >> >>
>>>>>     >> >> &sdio {
>>>>>     >> >> status = "okay";
>>>>>     >> >> vmmc-supply = <&wlan_en>;
>>>>>     >> >> bus-width = <4>;
>>>>>     >> >> max-frequency = <24000000>;
>>>>>     >> >> pinctrl-names = "default";
>>>>>     >> >> pinctrl-0 = <&sdio_pins>;
>>>>>     >> >> ti,non-removable;
>>>>>     >> >> ti,needs-special-hs-handling;
>>>>>     >> >> dma-ranges;
>>>>>     >> >> cap-power-off-card;
>>>>>     >> >> keep-power-in-suspend;
>>>>>     >> >>
>>>>>     >> >> #address-cells = <1>;
>>>>>     >> >> #size-cells = <0>;
>>>>>     >> >> wlcore: wlcore at 0 {
>>>>>     >> >> compatible = "ti,wl1835";
>>>>>     >> >> reg = <2>;
>>>>>     >> >> interrupt-parent = <&gpioa>;
>>>>>     >> >> interrupts = <8 IRQ_TYPE_EDGE_RISING>;
>>>>>     >> >> };
>>>>>     >> >> };
>>>>>     >> >>
>>>>>     >> >> >
>>>>>     >> >> > alex
>>>>>     >> >> >
>>>>>     >> >> >
>>>>>     >> >> >>
>>>>>     >> >> >>>
>>>>>     >> >> >>> Thanks in advance
>>>>>     >> >> >>> Alex
>>>>>     >> >> >>>
>>>>>     >> >> >>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>> Author: Bruno Herrera <bruherrera@gmail.com
>>>>>     <mailto:bruherrera@gmail.com>>
>>>>>
>>>>>     >> >> >>>> Date:   Sun Oct 16 14:50:00 2016 -0200
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>     ARM: DT: STM32: Use dma-ranges property per board
>>>>> not
>>>>>     at dtsi
>>>>>     >> >> >>>> file
>>>>>     >> >> >>>>
>>>>>     >> >> >>>> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>> b/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>> index 6bfc595..2a22a82 100644
>>>>>     >> >> >>>> --- a/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
>>>>>     >> >> >>>> @@ -52,6 +52,10 @@
>>>>>     >> >> >>>>         model = "STMicroelectronics STM32429i-EVAL
>>>>> board";
>>>>>     >> >> >>>>         compatible = "st,stm32429i-eval",
>>>>> "st,stm32f429";
>>>>>     >> >> >>>>
>>>>>     >> >> >>>> +       soc {
>>>>>     >> >> >>>> +               dma-ranges = <0xC0000000 0x0
>>>>> 0x10000000>;
>>>>>     >> >> >>>> +       };
>>>>>     >> >> >>>> +
>>>>>     >> >> >>>>         chosen {
>>>>>     >> >> >>>>                 bootargs = "root=/dev/ram
>>>>> rdinit=/linuxrc";
>>>>>     >> >> >>>>                 stdout-path = "serial0:115200n8";
>>>>>     >> >> >>>> @@ -96,6 +100,7 @@
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>  &ethernet0 {
>>>>>     >> >> >>>>         status = "okay";
>>>>>     >> >> >>>> +       dma-ranges;
>>>>>     >> >> >>>>         pinctrl-0       = <&ethernet0_mii>;
>>>>>     >> >> >>>>         pinctrl-names   = "default";
>>>>>     >> >> >>>>         phy-mode        = "mii-id";
>>>>>     >> >> >>>> @@ -116,6 +121,7 @@
>>>>>     >> >> >>>>  };
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>  &usbotg_hs {
>>>>>     >> >> >>>> +       dma-ranges;
>>>>>     >> >> >>>>         dr_mode = "host";
>>>>>     >> >> >>>>         phys = <&usbotg_hs_phy>;
>>>>>     >> >> >>>>         phy-names = "usb2-phy";
>>>>>     >> >> >>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>> index 7d624a2..697a133 100644
>>>>>     >> >> >>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>> @@ -59,7 +59,6 @@
>>>>>     >> >> >>>>         };
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>         soc {
>>>>>     >> >> >>>> -               dma-ranges = <0xc0000000 0x0
>>>>> 0x10000000>;
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>                 timer2: timer at 40000000 {
>>>>>     >> >> >>>>                         compatible = "st,stm32-timer";
>>>>>     >> >> >>>> @@ -472,13 +471,11 @@
>>>>>     >> >> >>>>                         st,syscon = <&syscfg 0x4>;
>>>>>     >> >> >>>>                         snps,pbl = <8>;
>>>>>     >> >> >>>>                         snps,mixed-burst;
>>>>>     >> >> >>>> -                       dma-ranges;
>>>>>     >> >> >>>>                         status = "disabled";
>>>>>     >> >> >>>>                 };
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>                 usbotg_hs: usb at 40040000 {
>>>>>     >> >> >>>>                         compatible = "snps,dwc2";
>>>>>     >> >> >>>> -                       dma-ranges;
>>>>>     >> >> >>>>                         reg = <0x40040000 0x40000>;
>>>>>     >> >> >>>>                         interrupts = <77>;
>>>>>     >> >> >>>>                         clocks = <&rcc 0 29>;
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>>> index 0596d60..3a1cfdd 100644
>>>>>     >> >> >>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>>>     >> >> >>>>> @@ -59,8 +59,6 @@
>>>>>     >> >> >>>>>         };
>>>>>     >> >> >>>>>
>>>>>     >> >> >>>>>         soc {
>>>>>     >> >> >>>>> -               dma-ranges = <0xc0000000 0x0
>>>>> 0x10000000>;
>>>>>     >> >> >>>>> -
>>>>>     >> >> >>>>>                 timer2: timer at 40000000 {
>>>>>     >> >> >>>>>                         compatible = "st,stm32-timer";
>>>>>     >> >> >>>>>                         reg = <0x40000000 0x400>;
>>>>>     >> >> >>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>     >> >> >>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>     >> >> >>>>> index 9e73656..c2213c0 100644
>>>>>     >> >> >>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>     >> >> >>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>     >> >> >>>>> @@ -64,6 +64,10 @@
>>>>>     >> >> >>>>>         aliases {
>>>>>     >> >> >>>>>                 serial0 = &usart3;
>>>>>     >> >> >>>>>         };
>>>>>     >> >> >>>>> +
>>>>>     >> >> >>>>> +       soc {
>>>>>     >> >> >>>>> +               dma-ranges = <0xc0000000 0x0
>>>>> 0x10000000>;
>>>>>     >> >> >>>>> +       };
>>>>>     >> >> >>>>>  };
>>>>>     >> >> >>>>>
>>>>>     >> >> >>>>>  &clk_hse {
>>>>>     >> >> >>>>> --
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>>
>>>>>     >> >> >>>> Br.,
>>>>>     >> >> >>>> Bruno
>>>>>     >> >> >>>>
>>>>>     >> >> >>>
>>>>>     >> >> >
>>>>>     >> >>
>>>>>     >> >> _______________________________________________
>>>>>     >> >> linux-arm-kernel mailing list
>>>>>     >> >> linux-arm-kernel at lists.infradead.org
>>>>>     <mailto:linux-arm-kernel@lists.infradead.org>
>>>>>     >> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>>>     <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>
>>>>>     >> >
>>>>>     >> >
>>>>>     >
>>>>>     >
>>>>>
>>>>>
>>>>
>>
>>

^ permalink raw reply

* [PATCH v5 5/7] net: ethernet: bgmac: device tree phy enablement
From: Rafal Milecki @ 2016-11-03  8:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478106488-11779-6-git-send-email-jon.mason@broadcom.com>

On 11/02/2016 06:08 PM, Jon Mason wrote:
> Change the bgmac driver to allow for phy's defined by the device tree

This is a late review, I know, sorry... :(


> +static int bcma_phy_direct_connect(struct bgmac *bgmac)
> +{
> +	struct fixed_phy_status fphy_status = {
> +		.link = 1,
> +		.speed = SPEED_1000,
> +		.duplex = DUPLEX_FULL,
> +	};
> +	struct phy_device *phy_dev;
> +	int err;
> +
> +	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
> +	if (!phy_dev || IS_ERR(phy_dev)) {
> +		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
> +		return -ENODEV;
> +	}
> +
> +	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
> +				 PHY_INTERFACE_MODE_MII);
> +	if (err) {
> +		dev_err(bgmac->dev, "Connecting PHY failed\n");
> +		return err;
> +	}
> +
> +	return err;
> +}

This bcma specific function looks exactly the same as...


> +static int platform_phy_direct_connect(struct bgmac *bgmac)
> +{
> +	struct fixed_phy_status fphy_status = {
> +		.link = 1,
> +		.speed = SPEED_1000,
> +		.duplex = DUPLEX_FULL,
> +	};
> +	struct phy_device *phy_dev;
> +	int err;
> +
> +	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
> +	if (!phy_dev || IS_ERR(phy_dev)) {
> +		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
> +		return -ENODEV;
> +	}
> +
> +	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
> +				 PHY_INTERFACE_MODE_MII);
> +	if (err) {
> +		dev_err(bgmac->dev, "Connecting PHY failed\n");
> +		return err;
> +	}
> +
> +	return err;
> +}

This one.

Would that make sense to keep bgmac_phy_connect_direct and just use it in
bcma/platform code?

^ permalink raw reply

* [PATCH v2 11/14] ASoC: sun4i-codec: Add support for A31 board level audio routing
From: Maxime Ripard @ 2016-11-03  8:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-12-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:53PM +0800, Chen-Yu Tsai wrote:
> The A31 SoC's codec has various inputs, outputs and microphone bias
> supplies. These can be routed on the board in different ways, such as:
> 
>   - HPCOM may be connected to have the headphone DC coupled.
> 
>   - Microphones all use the MBIAS main microphone supply or one mic may
>     use the HBIAS supply, which supports headset detection and buttons.
> 
>   - Line Out may be routed to an audio jack, or an onboard speaker amp
>     with power controls.
> 
> Add support for specifying the audio routes in the device tree.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/73c89dec/attachment.sig>

^ permalink raw reply

* [PATCH v2 10/14] ASoC: sun4i-codec: Add support for A31 ADC capture path
From: Maxime Ripard @ 2016-11-03  8:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-11-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:52PM +0800, Chen-Yu Tsai wrote:
> The A31's internal codec capture path has a mixer in front of the ADC
> for each channel, capable of selecting various inputs, including
> microphones, line in, phone in, and the main output mixer.
> 
> This patch adds the various controls, widgets and routes needed for
> audio capture from the already supported inputs on the A31.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/563cf894/attachment-0001.sig>

^ permalink raw reply

* [PATCH v2 07/14] ASoC: sun4i-codec: Add support for A31 Line In playback
From: Maxime Ripard @ 2016-11-03  8:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-8-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:49PM +0800, Chen-Yu Tsai wrote:
> The A31 integrated codec has a stereo "Line In" input. Add support for
> it to the playback paths.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/062cbad7/attachment.sig>

^ permalink raw reply

* [PATCH v2 05/14] ASoC: sun4i-codec: Add support for optional reset control to quirks
From: Maxime Ripard @ 2016-11-03  8:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-6-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:47PM +0800, Chen-Yu Tsai wrote:
> The later Allwinner SoCs have a dedicated reset controller, and
> peripherals have dedicated reset controls which need to be deasserted
> before the associated peripheral can be used.
> 
> Add support for this to the quirks structure and probe/remove functions.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/dcaed374/attachment.sig>

^ permalink raw reply

* [PATCH v2 04/14] ASoC: sun4i-codec: Increase DMA max burst to 8
From: Maxime Ripard @ 2016-11-03  8:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-5-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:46PM +0800, Chen-Yu Tsai wrote:
> According to the DMA engine API documentation, maxburst denotes the
> largest possible size of a single transfer, so as not to overflow
> destination FIFOs as explained in this excerpt from dmaengine.h
> 
>  * @src_maxburst: the maximum number of words (note: words, as in
>  * units of the src_addr_width member, not bytes) that can be sent
>  * in one burst to the device. Typically something like half the
>  * FIFO depth on I/O peripherals so you don't overflow it. This
>  * may or may not be applicable on memory sources.
>  * @dst_maxburst: same as src_maxburst but for destination target
>  * mutatis mutandis.
> 
> The TX FIFO is 64 samples deep for stereo, and the RX FIFO is 16
> samples deep. So maxburst could be 32 and 8 for TX and RX respectively.
> 
> Unfortunately the sunxi DMA controller driver takes maxburst as
> the requested burst size, rather than a limit, and returns an error
> for unsupported values. The original value was 4, but some later
> SoCs do not officially support this burst size.
> 
> This patch increases maxburst on the TX side to 8, which is supported
> by all variants of the sunxi DMA controller.
> 
> Cc: Vinod Koul <vinod.koul@intel.com>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/7e1feb49/attachment.sig>

^ permalink raw reply

* [PATCH v2 03/14] ASoC: sun4i-codec: Revise comments for register definition macros
From: Maxime Ripard @ 2016-11-03  8:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-4-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:45PM +0800, Chen-Yu Tsai wrote:
> This revises existing comments in the register definition macros
> section, and adds a few more, so that readers can clearly identify
> the types of control registers.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/15ffa371/attachment.sig>

^ permalink raw reply

* [PATCH 0/3] fix ohci phy name
From: Axel Haslam @ 2016-11-03  8:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161102124435.31777-1-ahaslam@baylibre.com>

Hi Sekhar, David,

It might make sense to have this patch series,
squashed into a single patch, would you agree,
or do you prefer it as is: one-per-subsystem?

Regards
Axel.

On Wed, Nov 2, 2016 at 1:44 PM, Axel Haslam <ahaslam@baylibre.com> wrote:
> The usb ohci clock match is not working because the usb clock
> is registered as "ohci" instead of "ohci.0"
>
> But since there is only a single ohci instance, lets pass -1 to
> the platform data id parameter and avoid the extra ".0" matching.
>
> while we are fixing this, rename the driver to "ohci-da8xx" to be
> consistent with davinci musb and other usb drivers.
>
> Axel Haslam (3):
>   ARM: davinci: da8xx: Fix ohci driver name
>   phy: da8xx-usb: rename the ohci device to ohci-da8xx
>   usb: ohci-da8xx: rename driver to ohci-da8xx
>
>  arch/arm/mach-davinci/da830.c     | 2 +-
>  arch/arm/mach-davinci/da850.c     | 2 +-
>  arch/arm/mach-davinci/da8xx-dt.c  | 2 +-
>  arch/arm/mach-davinci/usb-da8xx.c | 4 ++--
>  drivers/phy/phy-da8xx-usb.c       | 5 +++--
>  drivers/usb/host/ohci-da8xx.c     | 2 +-
>  6 files changed, 9 insertions(+), 8 deletions(-)
>
> --
> 2.10.1
>

^ permalink raw reply

* [PATCH v2 01/14] ASoC: sun4i-codec: Move data structures to add create_card call to quirks
From: Maxime Ripard @ 2016-11-03  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103075556.29018-2-wens@csie.org>

On Thu, Nov 03, 2016 at 03:55:43PM +0800, Chen-Yu Tsai wrote:
> The audio codec on later Allwinner SoCs have a different layout and
> audio path compared to the A10/A20. However the PCM parts are still
> the same.
> 
> The different layout and audio paths mean we need a different
> create_card function for different families, so they can create
> DAPM endpoint widgets and routes.
> 
> This patch moves the regmap configs, quirks and of_device_id
> structures to just before the probe function, so we can, among other
> things, include a pointer for the create_card function. None of the
> lines of code were changed.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/80d51ad8/attachment.sig>

^ permalink raw reply

* [PATCH 02/10] iio: adc: Add stm32 support
From: Fabrice Gasnier @ 2016-11-03  8:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <a1445535-5f86-e95d-ed38-03edb8c9f9d8@kernel.org>

On 10/30/2016 04:27 PM, Jonathan Cameron wrote:
> On 25/10/16 17:25, Fabrice Gasnier wrote:
>> This patch adds support for STMicroelectronics STM32 MCU's analog to
>> digital converter.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Hi Fabrice,
>
> Sometimes I hate SoC ADCs.  For some reason the hardware designers seem to
> try and throw everything and the kitchen sink at them.  Discontinuous mode
> as an example in this device.  Not seen that particular piece of fun before
> and glad to see you haven't 'yet' tried to support it!
>
> Anyhow, the complexity of the hardware leads to an initially complex driver.
> My first thought it that this would be easier to follow / review if we
> built it up in smaller steps.   Perhaps ditch the injected channel support
> entirely in the first instance.  I also wonder if you don't need to support
> that whole thing (injected sampling) as another iio device entirely using the
> same channels.  That's kind of what it is from a data flow point of view
> (we've had arbitary sequencers before with priorities - don't think anyone
> ever decided the pain was worth supporting the complexity, but right answer
> has always been multiple IIO devices).
Hi Jonathan,

First, many thanks for your review. I agree with you, most reasonable 
approach is to remove some
complexity to ease the review. Regarding injected support, basically, 
bellow approach is to use
separate IIO devices for regular and injected. But, I'll remove this, at 
least for now, in next patch set.

> You also have at least one layer of abstraction in here that serves no
> current purpose.  Please clear that out for now. It'll make the code
> shorter and easier to follow.  If/when other parts are introduced then
> is the time to do that transistion to having the abstraction.

 From your suggestion, this may end-up in a single driver file in 
drivers/iio.
I think I'll try to keep simple routines like start, stop, conf_scan and 
so on, but
remove indirection routines from stm32-adc.h file (e.g. stm32_adc_ops).
Is it in line with your suggestions ?

>
> My first thought on the double / tripple adc handling is that you'd be better
> off handling them as 3 separate devices then doing some 'unusual' trigger
> handling to support the weird sequencing.  Guessing you thought about that?
> If so could you lay out your reasoning for the single driver instance approach.
> I'm not arguing against it btw, merely want to understand your reasoning!

I mainly came up with a single driver instance approach because there 
are basically
3 identical ADC instances 'mapped' in a single IP with few common resources.
I usually see mfd are more heterogeneous and declare cells for various 
subsystem drivers.
But I can try to move to mfd as you're suggesting.
I just hope this will not bring more complexity.

>
> It would be tricky given one set of channels are selectable over 3 devices
> and there are constraints to enforce (not sampling same channel on two ADCs
> at the same time) but not impossible...  Perhaps what you have here is
> indeed simpler!
>
> Whilst it's been a nasty job to review, I'm guessing writing it was
> much worse ;)  Pretty good starting point though might take a little while
> to pin down the remaining questions on how best to handle this particular
> monster.
My apologies... I hope you didn't had much of a headache :-) by reading me.
More questions bellow.

> Jonathan
>> ---
>>   drivers/iio/adc/Kconfig             |   2 +
>>   drivers/iio/adc/Makefile            |   1 +
>>   drivers/iio/adc/stm32/Kconfig       |  34 ++
>>   drivers/iio/adc/stm32/Makefile      |   4 +
>>   drivers/iio/adc/stm32/stm32-adc.c   | 999 ++++++++++++++++++++++++++++++++++++
>>   drivers/iio/adc/stm32/stm32-adc.h   | 442 ++++++++++++++++
>>   drivers/iio/adc/stm32/stm32f4-adc.c | 574 +++++++++++++++++++++
>>   7 files changed, 2056 insertions(+)
>>   create mode 100644 drivers/iio/adc/stm32/Kconfig
>>   create mode 100644 drivers/iio/adc/stm32/Makefile
>>   create mode 100644 drivers/iio/adc/stm32/stm32-adc.c
>>   create mode 100644 drivers/iio/adc/stm32/stm32-adc.h
>>   create mode 100644 drivers/iio/adc/stm32/stm32f4-adc.c
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 7edcf32..5c96a55 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -583,4 +583,6 @@ config XILINX_XADC
>>   	  The driver can also be build as a module. If so, the module will be called
>>   	  xilinx-xadc.
>>   
>> +source "drivers/iio/adc/stm32/Kconfig"
>> +
>>   endmenu
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index 7a40c04..a9dbf3a 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -13,6 +13,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
>>   obj-$(CONFIG_AD7793) += ad7793.o
>>   obj-$(CONFIG_AD7887) += ad7887.o
>>   obj-$(CONFIG_AD799X) += ad799x.o
>> +obj-$(CONFIG_ARCH_STM32) += stm32/
>>   obj-$(CONFIG_AT91_ADC) += at91_adc.o
>>   obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
>>   obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
>> diff --git a/drivers/iio/adc/stm32/Kconfig b/drivers/iio/adc/stm32/Kconfig
>> new file mode 100644
>> index 0000000..245d037
>> --- /dev/null
>> +++ b/drivers/iio/adc/stm32/Kconfig
>> @@ -0,0 +1,34 @@
>> +#
>> +# STM32 familly ADC drivers
>> +#
>> +
>> +config STM32_ADC
>> +	tristate
>> +	select REGULATOR
>> +	select REGULATOR_FIXED_VOLTAGE
>> +	select IIO_BUFFER
>> +	select IIO_TRIGGERED_BUFFER
>> +	help
>> +	  Say yes here to build the driver for the STMicroelectronics
>> +	  STM32 analog-to-digital converter (ADC).
>> +
>> +	  This driver can also be built as a module.  If so, the module
>> +	  will be called stm32-adc.
>> +
>> +config STM32F4_ADC
>> +	tristate "STMicroelectronics STM32F4 adc"
>> +	depends on ARCH_STM32 || COMPILE_TEST
>> +	depends on OF
>> +	select STM32_ADC
>> +	help
>> +	  Say yes here to build support for STMicroelectronics stm32f4 Analog
>> +	  to Digital Converter (ADC).
>> +
>> +	  This driver can also be built as a module.  If so, the module
>> +	  will be called stm32f4-adc.
>> +
>> +config STM32_ADC_DEBUG
>> +	bool "Enable debug for stm32 ADC drivers"
>> +	depends on STM32_ADC
>> +	help
>> +	  Say "yes" to enable debug messages, on stm32 ADC drivers.
>> diff --git a/drivers/iio/adc/stm32/Makefile b/drivers/iio/adc/stm32/Makefile
>> new file mode 100644
>> index 0000000..83e8154
>> --- /dev/null
>> +++ b/drivers/iio/adc/stm32/Makefile
>> @@ -0,0 +1,4 @@
>> +# Core
>> +subdir-ccflags-$(CONFIG_STM32_ADC_DEBUG) := -DDEBUG
>> +obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>> +obj-$(CONFIG_STM32F4_ADC) += stm32f4-adc.o
>> diff --git a/drivers/iio/adc/stm32/stm32-adc.c b/drivers/iio/adc/stm32/stm32-adc.c
>> new file mode 100644
>> index 0000000..1e0850d
>> --- /dev/null
>> +++ b/drivers/iio/adc/stm32/stm32-adc.c
>> @@ -0,0 +1,999 @@
[snip]

>> +
>> +static int stm32_adc_conf_scan(struct iio_dev *indio_dev,
>> +			       const unsigned long *scan_mask)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	ret = stm32_adc_clk_sel(adc);
>> +	if (ret) {
>> +		dev_err(&indio_dev->dev, "Clock sel failed\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = stm32_adc_enable(adc);
>> +	if (ret) {
>> +		dev_err(&indio_dev->dev, "Failed to enable adc\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask);
>> +	if (ret) {
>> +		dev_err(&indio_dev->dev, "Failed to configure sequence\n");
>> +		goto err_dis;
>> +	}
> It's horrible but to end up in the 'obvious' state I'd disable the adc
> again assuming that doesn't kill the stuff that is configured.
I'll check this and try to come up with something.

>> +
>> +	return 0;
>> +
>> +err_dis:
>> +	stm32_adc_disable(adc);
>> +
>> +	return ret;
>> +}
>> +
[snip]
>> +/**
>> + * stm32_adc_single_conv() - perform a single conversion
>> + * @indio_dev: IIO device
>> + * @chan: IIO channel
>> + * @result: conversion result
>> + *
>> + * The function performs a single conversion on a given channel, by
>> + * by:
>> + * - creating scan mask with only one channel
>> + * - using SW trigger
>> + * - then start single conv
>> + */
>> +static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>> +				 const struct iio_chan_spec *chan,
>> +				 int *val)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	unsigned long *scan_mask;
>> +	long timeout;
>> +	u16 result;
>> +	int ret;
>> +
>> +	scan_mask = kcalloc(BITS_TO_LONGS(indio_dev->masklength), sizeof(long),
>> +			    GFP_KERNEL);
> This is known maximum length... I'd just avoid the complexity of allocating
> it like this - a comment would do the job to say it is the right length.
Do you suggest to use a predefined variable (like unsigned long 
scan_mask) directly ?
And add a more basic test on 'masklength', to be sure ?

>> +	if (!scan_mask)
>> +		return -ENOMEM;
>> +
>> +	set_bit(chan->scan_index, scan_mask);
>> +
>> +	reinit_completion(&adc->completion);
>> +
>> +	adc->bufi = 0;
>> +	adc->num_conv = 1;
>> +	adc->buffer = &result;
>> +
>> +	ret = stm32_adc_conf_scan(indio_dev, scan_mask);
>> +	if (ret)
>> +		goto free;
>> +
>> +	/* No HW trigger: conversion can be launched in SW */
>> +	ret = stm32_adc_set_trig(indio_dev, NULL);
> Put it back again afterwards?  Otherwise some nasty race conditions look
> likely to me.. (userspace sets trigger and is about to enable the buffer
> when along comes this code and changes it underneath).
I'll fix this.

>> +	if (ret) {
>> +		dev_err(&indio_dev->dev, "Can't set SW trigger\n");
>> +		goto adc_disable;
>> +	}
>> +
>> +	stm32_adc_conv_irq_enable(adc);
>> +
>> +	ret = stm32_adc_start_conv(adc);
>> +	if (ret) {
>> +		dev_err(&indio_dev->dev, "Failed to start single conv\n");
>> +		goto irq_disable;
>> +	}
>> +
>> +	timeout = wait_for_completion_interruptible_timeout(
>> +					&adc->completion, STM32_ADC_TIMEOUT);
>> +	if (timeout == 0) {
>> +		dev_warn(&indio_dev->dev, "Conversion timed out!\n");
>> +		ret = -ETIMEDOUT;
>> +	} else if (timeout < 0) {
>> +		dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
>> +		ret = -EINTR;
>> +	} else {
>> +		*val = result & STM32_RESULT_MASK;
>> +		ret = IIO_VAL_INT;
>> +	}
>> +
>> +	if (stm32_adc_stop_conv(adc))
>> +		dev_err(&indio_dev->dev, "stop failed\n");
>> +
>> +irq_disable:
>> +	stm32_adc_conv_irq_disable(adc);
>> +
>> +adc_disable:
>> +	stm32_adc_disable(adc);
>> +
>> +free:
>> +	kfree(scan_mask);
>> +	adc->buffer = NULL;
>> +
>> +	return ret;
>> +}
>> +
>> +static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>> +			      struct iio_chan_spec const *chan,
>> +			      int *val, int *val2, long mask)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	int ret = -EINVAL;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		ret = iio_device_claim_direct_mode(indio_dev);
>> +		if (ret)
>> +			return ret;
>> +		if (chan->type == IIO_VOLTAGE)
>> +			ret = stm32_adc_single_conv(indio_dev, chan, val);
>> +		iio_device_release_direct_mode(indio_dev);
>> +		break;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = adc->common->vref_mv;
>> +		*val2 = chan->scan_type.realbits;
>> +		ret = IIO_VAL_FRACTIONAL_LOG2;
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +/**
>> + * stm32_adc_isr() - Treat interrupt for one ADC instance within ADC block
> As this is kernel doc please document the parameter as well.  Otherwise
> we'll get a pile of warnings!
Sure.
>> + */
>> +static irqreturn_t stm32_adc_isr(struct stm32_adc *adc)
>> +{
>> +	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>> +	const struct stm32_adc_reginfo *reginfo =
>> +		adc->common->data->adc_reginfo;
>> +	u32 mask, clr_mask, status = stm32_adc_readl(adc, reginfo->isr);
>> +
>> +	if (adc->injected) {
>> +		mask = reginfo->jeoc;
>> +		clr_mask = mask;
>> +	} else {
>> +		mask = reginfo->eoc;
>> +		/* don't clear 'eoc' as it is cleared when reading 'dr' */
>> +		clr_mask = 0;
>> +	}
>> +
>> +	/* clear irq */
>> +	stm32_adc_writel(adc, reginfo->isr, status & ~clr_mask);
> Want to do this in the non injected case? it's a noop isn't it?
I'll rework this.

>
>> +	status &= mask;
>> +
>> +	/* Regular data */
>> +	if (status & reginfo->eoc) {
> Hmm.. this is a little bit of 'missuse' of the standard trigger architecture
> but as long as it's restricted to just this device I don't suppose we need
> to care.  Only reason we need it is to provide control of 'which' hardware
> trigger is being used.
>
> Guessing the DMA will almost always be turned on and will make this oddity
> effectively disappear.
I'm not sure to understand your remark. Above test checks end of 
conversion status flag.
Or do you talk about bellow lines ? Can you please clarify ?

>> +		adc->buffer[adc->bufi] = stm32_adc_readl(adc, reginfo->dr);
>> +		if (iio_buffer_enabled(indio_dev)) {
>> +			adc->bufi++;
>> +			if (adc->bufi >= adc->num_conv) {
>> +				stm32_adc_conv_irq_disable(adc);
>> +				iio_trigger_poll(indio_dev->trig);
>> +			}
>> +		} else {
>> +			complete(&adc->completion);
>> +		}
>> +	}
>> +
>> +	/* Injected data */
>> +	if (status & reginfo->jeoc) {
>> +		int i;
>> +
>> +		for (i = 0; i < adc->num_conv; i++) {
>> +			adc->buffer[i] = stm32_adc_readl(adc, reginfo->jdr[i]);
>> +			adc->bufi++;
>> +		}
>> +
>> +		if (iio_buffer_enabled(indio_dev)) {
>> +			stm32_adc_conv_irq_disable(adc);
>> +			iio_trigger_poll(indio_dev->trig);
>> +		} else {
>> +			complete(&adc->completion);
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * In case end of conversion flags have been handled, this has been
>> +	 * handled for this ADC instance
>> +	 */
>> +	if (status)
>> +		return IRQ_HANDLED;
>> +
>> +	/* This adc instance didn't trigger this interrupt */
>> +	return IRQ_NONE;
>> +}
>> +
>> +/**
>> + * stm32_adc_common_isr() - Common isr for the whole ADC block
>> + *
>> + * There is one IRQ for all ADCs in ADC block, check all instances.
>> + */
>> +static irqreturn_t stm32_adc_common_isr(int irq, void *data)
>> +{
>> +	struct stm32_adc_common *common = data;
>> +	irqreturn_t ret = IRQ_NONE;
>> +	struct stm32_adc *adc;
>> +
>> +	list_for_each_entry(adc, &common->adc_list, adc_list)
>> +		ret |= stm32_adc_isr(adc);
> Hmm.. ret |= is rather fragile.  Preferable to make the handling of NONE
> vs IRQ_HANDLED explicit.
>
> If you were to split the driver up as I suggested might make sense above,
> then this would be done with an irq chip in a top level device (effectively
> a very simple mfd).
I'll look into it.

>> +
>> +	return ret;
>> +}
>> +
>> +/**
>> + * stm32_adc_validate_trigger() - validate trigger for stm32 adc
>> + * @indio_dev: IIO device
>> + * @trig: new trigger
>> + *
>> + * Returns: 0 if trig matches one of the triggers registered by stm32 adc
>> + * driver, -EINVAL otherwise.
>> + */
>> +static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>> +				      struct iio_trigger *trig)
>> +{
>> +	return stm32_adc_get_trig_index(indio_dev, trig) < 0 ? -EINVAL : 0;
>> +}
>> +
>> +static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
>> +				      const unsigned long *scan_mask)
> I'm glad you kept this relatively simple compared to some of the
> 'fun' the hardware is capable of. Very wise!
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	int ret;
>> +	u32 bit;
>> +
>> +	adc->num_conv = 0;
>> +	for_each_set_bit(bit, scan_mask, indio_dev->masklength)
>> +		adc->num_conv++;
>> +
>> +	ret = stm32_adc_conf_scan(indio_dev, scan_mask);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
>> +			      const struct of_phandle_args *iiospec)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < indio_dev->num_channels; i++)
>> +		if (indio_dev->channels[i].channel == iiospec->args[0])
>> +			return i;
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +/**
>> + * stm32_adc_debugfs_reg_access - read or write register value
>> + *
>> + * To read a value from an ADC register:
>> + *   echo [ADC reg offset] > direct_reg_access
>> + *   cat direct_reg_access
>> + *
>> + * To write a value in a ADC register:
>> + *   echo [ADC_reg_offset] [value] > direct_reg_access
>> + */
>> +static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>> +					unsigned reg, unsigned writeval,
>> +					unsigned *readval)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +
>> +	if (!readval)
>> +		stm32_adc_writel(adc, reg, writeval);
>> +	else
>> +		*readval = stm32_adc_readl(adc, reg);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct iio_info stm32_adc_iio_info = {
>> +	.read_raw = stm32_adc_read_raw,
>> +	.validate_trigger = stm32_adc_validate_trigger,
>> +	.update_scan_mode = stm32_adc_update_scan_mode,
>> +	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
>> +	.of_xlate = stm32_adc_of_xlate,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static int stm32_adc_buffer_postdisable(struct iio_dev *indio_dev)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +
>> +	stm32_adc_disable(adc);
> This is a surprise as postdisbale should balance preenable...
> Ah, you have update scan mode enabling the adc.  If you can balance it
> better by moving that to preenable please do as it is more 'obviously' correct.
I'll try to rework this.
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
>> +	.postenable = &iio_triggered_buffer_postenable,
>> +	.predisable = &iio_triggered_buffer_predisable,
>> +	.postdisable = &stm32_adc_buffer_postdisable,
>> +};
>> +
>> +static int stm32_adc_validate_device(struct iio_trigger *trig,
>> +				     struct iio_dev *indio_dev)
>> +{
>> +	struct iio_dev *indio = iio_trigger_get_drvdata(trig);
>> +
>> +	return indio != indio_dev ? -EINVAL : 0;
>> +}
>> +
>> +static int stm32_adc_set_trigger_state(struct iio_trigger *trig,
>> +				       bool state)
>> +{
>> +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	if (state) {
>> +		/* Reset adc buffer index */
>> +		adc->bufi = 0;
>> +
>> +		/* Allocate adc buffer */
>> +		adc->buffer = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
> I'd be more cynical.  It's not that big a memory allocation at worst.
> Just put a big enough buffer in your adc structure and don't bother doing
> it dynamically.
>
> If you didn't want to do it, it should be in the preenable callback rather
> than the trigger state one (for semantic reasons rather than because it's a
> bug)
I'll fix this.
>> +		if (!adc->buffer)
>> +			return -ENOMEM;
>> +
>> +		ret = stm32_adc_set_trig(indio_dev, trig);
>> +		if (ret) {
>> +			dev_err(&indio_dev->dev, "Can't set trigger\n");
>> +			goto err_buffer_free;
>> +		}
>> +
>> +		stm32_adc_conv_irq_enable(adc);
>> +
>> +		ret = stm32_adc_start_conv(adc);
>> +		if (ret) {
>> +			dev_err(&indio_dev->dev, "Failed to start\n");
>> +			goto err_irq_trig_disable;
>> +		}
>> +	} else {
>> +		ret = stm32_adc_stop_conv(adc);
>> +		if (ret < 0) {
>> +			dev_err(&indio_dev->dev, "Failed to stop\n");
>> +			return ret;
>> +		}
>> +
>> +		stm32_adc_conv_irq_disable(adc);
>> +
>> +		ret = stm32_adc_set_trig(indio_dev, NULL);
>> +		if (ret)
>> +			dev_warn(&indio_dev->dev, "Can't clear trigger\n");
>> +
>> +		kfree(adc->buffer);
>> +		adc->buffer = NULL;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_irq_trig_disable:
>> +	stm32_adc_conv_irq_disable(adc);
>> +	stm32_adc_set_trig(indio_dev, NULL);
>> +
>> +err_buffer_free:
>> +	kfree(adc->buffer);
>> +	adc->buffer = NULL;
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct iio_trigger_ops stm32_adc_trigger_ops = {
>> +	.owner = THIS_MODULE,
>> +	.validate_device = stm32_adc_validate_device,
>> +	.set_trigger_state = stm32_adc_set_trigger_state,
>> +};
>> +
>> +static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>> +{
>> +	struct iio_poll_func *pf = p;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +
>> +	dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
>> +
>> +	/* reset buffer index */
>> +	adc->bufi = 0;
>> +	iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
>> +					   pf->timestamp);
>> +
>> +	iio_trigger_notify_done(indio_dev->trig);
>> +
>> +	/* re-enable eoc irq */
>> +	stm32_adc_conv_irq_enable(adc);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static void stm32_adc_trig_unregister(struct iio_dev *indio_dev)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	struct iio_trigger *trig, *_t;
>> +
>> +	list_for_each_entry_safe(trig, _t, &adc->extrig_list, alloc_list) {
>> +		iio_trigger_unregister(trig);
>> +		list_del(&trig->alloc_list);
>> +	}
>> +}
>> +
> I'd like a bit of documentation on this and a few of the other more
> complex functions.  Here it wasn't immediately obvious to me that it
> was registering a large set of triggers.  Also, silly question but
> do you have any means of controlling the various timer setups from userspace?
Sorry about this, I'll try to comment about trigger list to make it more 
obvious.
There is no mean to setup timers via userspace, yet...
I can remove them from the list for now, until this is supported.
BTW I have some questions on trigger...

>
> There have been numerous discussions over the years on having a generic
> timer subsystem, but if anything got written it passed me by. I have a couple
> of boards where it would be handy but never had the time to do more than
> talk about it ;)

This is interesting... I'd be glad to hear more about it. Can you point 
some discussions if you have it in mind?

In this driver, validate_trigger routine enforces that only triggers 
allocated for current indio_dev can be used.
What if all timer triggers are put in a separate driver ? (e.g. like 
hrtimer in drivers/iio/trigger/) ?
Purpose would be to tune 'sampling_frequency' and so on, on similar 
model, and have it configured basically when using it
(e.g. cat trigger/name>trigger/current_trigger.).

Is it a viable option, not to declare timer triggers in stm32-adc.c, but 
use pre-defined list of triggers, and separate trigger driver ?
I'm thinking then, of simple string based list... But maybe you already 
though about this king of things ?

Please kindly share you view on this.

Thanks again for your review.
Best Regards,
Fabrice

>> +static int stm32_adc_trig_register(struct iio_dev *indio_dev)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	struct stm32_adc_common *common = adc->common;
>> +	const struct stm32_adc_trig_info *ext = common->data->ext_triggers;
>> +	struct iio_trigger *trig;
>> +	int i, ret = 0;
>> +
>> +	if (adc->injected)
>> +		ext = common->data->jext_triggers;
>> +	else
>> +		ext = common->data->ext_triggers;
>> +
>> +	for (i = 0; ext && ext[i].name; i++) {
>> +		trig = devm_iio_trigger_alloc(common->dev, "%s_%s%d_%s",
>> +					      indio_dev->name,
>> +					      adc->injected ? "jext" : "ext",
>> +					      ext[i].extsel, ext[i].name);
>> +		if (!trig) {
>> +			dev_err(common->dev, "trig %s_%s%d_%s alloc failed\n",
>> +				indio_dev->name,
>> +				adc->injected ? "jext" : "ext",
>> +				ext[i].extsel, ext[i].name);
>> +			ret = -ENOMEM;
>> +			goto err;
>> +		}
>> +
>> +		trig->dev.parent = common->dev;
>> +		trig->ops = &stm32_adc_trigger_ops;
>> +		iio_trigger_set_drvdata(trig, indio_dev);
>> +
>> +		ret = iio_trigger_register(trig);
>> +		if (ret) {
>> +			dev_err(common->dev,
>> +				"trig %s_%s%d_%s register failed\n",
>> +				indio_dev->name,
>> +				adc->injected ? "jext" : "ext",
>> +				ext[i].extsel, ext[i].name);
>> +			goto err;
>> +		}
>> +
>> +		list_add_tail(&trig->alloc_list, &adc->extrig_list);
>> +	}
>> +
>> +	return 0;
>> +err:
>> +	stm32_adc_trig_unregister(indio_dev);
>> +
>> +	return ret;
>> +}
>> +
>> +static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> +				    struct iio_chan_spec *chan,
>> +				    const struct stm32_adc_chan_spec *channel,
>> +				    int scan_index)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +
>> +	chan->type = channel->type;
>> +	chan->channel = channel->channel;
>> +	chan->datasheet_name = channel->name;
>> +	chan->extend_name = channel->name;
>> +	chan->scan_index = scan_index;
>> +	chan->indexed = 1;
>> +	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
>> +	chan->scan_type.sign = 'u';
>> +	chan->scan_type.realbits = adc->common->data->highres;
>> +	chan->scan_type.storagebits = STM32_STORAGEBITS;
> This is one of those cases where actually I'd argue just having the number
> here and not under a define would be clearer!  So just put 16 here.
>> +	chan->scan_type.shift = 0;
> Should be unneeded.  Shift of 0 is the obvious default so no info provided
> to readers of the code either really.
I'll fix this
>> +}
>> +
>> +static int stm32_adc_chan_of_init(struct iio_dev *indio_dev,
>> +				  const struct stm32_adc_info *adc_info)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	struct device_node *node = indio_dev->dev.of_node;
>> +	struct property *prop;
>> +	const __be32 *cur;
>> +	struct iio_chan_spec *channels;
>> +	int scan_index = 0, num_channels = 0;
>> +	u32 val;
>> +
>> +	of_property_for_each_u32(node, "st,adc-channels", prop, cur, val)
>> +		num_channels++;
>> +
>> +	channels = devm_kcalloc(&indio_dev->dev, num_channels,
>> +				sizeof(struct iio_chan_spec), GFP_KERNEL);
>> +	if (!channels)
>> +		return -ENOMEM;
>> +
>> +	of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> +					&adc_info->channels[val],
>> +					scan_index);
>> +		scan_index++;
>> +	}
>> +
>> +	adc->max_channels = adc_info->max_channels;
>> +	indio_dev->num_channels = scan_index;
>> +	indio_dev->channels = channels;
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_adc_register(struct stm32_adc_common *common,
>> +			      struct device_node *child)
>> +{
>> +	struct iio_dev *indio_dev;
>> +	struct stm32_adc *adc;
>> +	int i, ret;
>> +	u32 reg;
>> +
>> +	ret = of_property_read_u32(child, "reg", &reg);
>> +	if (ret != 0) {
>> +		dev_err(common->dev, "missing reg property\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	for (i = 0; common->data->adc_info[i].channels; i++)
>> +		if (common->data->adc_info[i].reg == reg)
>> +			break;
>> +
>> +	if (i >= STM32_ADC_ID_MAX || !common->data->adc_info[i].channels) {
>> +		dev_err(common->dev, "bad adc reg offset\n");
>> +		return -ENOENT;
>> +	}
>> +
>> +	indio_dev = devm_iio_device_alloc(common->dev, sizeof(*adc));
>> +	if (!indio_dev) {
>> +		dev_err(common->dev, "iio device allocation failed\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	adc = iio_priv(indio_dev);
>> +	adc->id = i;
>> +	adc->offset = reg;
>> +	adc->common = common;
>> +	INIT_LIST_HEAD(&adc->extrig_list);
>> +	spin_lock_init(&adc->lock);
>> +	init_completion(&adc->completion);
>> +
>> +	if (child->name)
>> +		indio_dev->name = child->name;
>> +	else
>> +		indio_dev->name = common->data->adc_info[i].name;
>> +	indio_dev->dev.parent = common->dev;
>> +	indio_dev->dev.of_node = child;
>> +	indio_dev->info = &stm32_adc_iio_info;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	if (of_property_read_bool(child, "st,injected")) {
>> +		dev_dbg(common->dev, "%s Configured to use injected\n",
>> +			indio_dev->name);
>> +		adc->injected = true;
>> +	}
>> +
>> +	adc->clk = of_clk_get(child, 0);
>> +	if (IS_ERR(adc->clk)) {
>> +		adc->clk = NULL;
>> +		dev_dbg(common->dev, "No child clk found\n");
>> +	} else {
>> +		ret = clk_prepare_enable(adc->clk);
>> +		if (ret < 0)
>> +			goto err_clk_put;
>> +	}
>> +
>> +	ret = stm32_adc_chan_of_init(indio_dev, &common->data->adc_info[i]);
>> +	if (ret < 0) {
>> +		dev_err(common->dev, "iio channels init failed\n");
>> +		goto err_clk_disable;
>> +	}
>> +
>> +	ret = stm32_adc_trig_register(indio_dev);
>> +	if (ret)
>> +		goto err_clk_disable;
>> +
>> +	ret = iio_triggered_buffer_setup(indio_dev,
>> +					 &iio_pollfunc_store_time,
>> +					 &stm32_adc_trigger_handler,
>> +					 &iio_triggered_buffer_setup_ops);
>> +	if (ret) {
>> +		dev_err(common->dev, "buffer setup failed\n");
>> +		goto err_trig_unregister;
>> +	}
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret) {
>> +		dev_err(common->dev, "iio dev register failed\n");
>> +		goto err_buffer_cleanup;
>> +	}
>> +
>> +	list_add_tail(&adc->adc_list, &common->adc_list);
>> +
>> +	return 0;
>> +
>> +err_buffer_cleanup:
>> +	iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +err_trig_unregister:
>> +	stm32_adc_trig_unregister(indio_dev);
>> +
>> +err_clk_disable:
>> +	if (adc->clk)
>> +		clk_disable_unprepare(adc->clk);
>> +
>> +err_clk_put:
>> +	if (adc->clk)
>> +		clk_put(adc->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static void stm32_adc_unregister(struct stm32_adc *adc)
>> +{
>> +	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>> +
>> +	iio_device_unregister(indio_dev);
>> +	iio_triggered_buffer_cleanup(indio_dev);
>> +	stm32_adc_trig_unregister(indio_dev);
>> +	if (adc->clk) {
>> +		clk_disable_unprepare(adc->clk);
>> +		clk_put(adc->clk);
>> +	}
>> +}
>> +
>> +int stm32_adc_probe(struct platform_device *pdev)
>> +{
>> +	struct device_node *np = pdev->dev.of_node, *child;
>> +	struct device *dev = &pdev->dev;
>> +	const struct of_device_id *match;
>> +	struct stm32_adc_common *common;
>> +	struct stm32_adc *adc;
>> +	struct resource *res;
>> +	int ret;
>> +
>> +	match = of_match_device(dev->driver->of_match_table, &pdev->dev);
>> +	if (!match || !match->data) {
>> +		dev_err(&pdev->dev, "compatible data not provided\n");
> How would we have instantiated this if there was not a suitable match?
> As such what does this check give us? (confused!)
I'll fix this.
>> +		return -EINVAL;
>> +	}
>> +
>> +	common = devm_kzalloc(&pdev->dev, sizeof(*common), GFP_KERNEL);
>> +	if (!common)
>> +		return -ENOMEM;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	common->base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(common->base))
>> +		return PTR_ERR(common->base);
>> +
>> +	common->data = match->data;
>> +	common->dev = &pdev->dev;
>> +	platform_set_drvdata(pdev, common);
>> +	mutex_init(&common->lock);
>> +	INIT_LIST_HEAD(&common->adc_list);
>> +
>> +	common->vref = devm_regulator_get(&pdev->dev, "vref");
>> +	if (IS_ERR(common->vref)) {
>> +		ret = PTR_ERR(common->vref);
>> +		dev_err(&pdev->dev, "vref get failed, %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = regulator_enable(common->vref);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "vref enable failed\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = regulator_get_voltage(common->vref);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
>> +		goto err_regulator_disable;
>> +	}
>> +	common->vref_mv = ret / 1000;
>> +	dev_dbg(&pdev->dev, "vref+=%dmV\n", common->vref_mv);
>> +
>> +	common->aclk = devm_clk_get(&pdev->dev, "adc");
>> +	if (IS_ERR(common->aclk)) {
>> +		ret = PTR_ERR(common->aclk);
>> +		dev_err(&pdev->dev, "Can't get 'adc' clock\n");
>> +		goto err_regulator_disable;
>> +	}
>> +
>> +	ret = clk_prepare_enable(common->aclk);
>> +	if (ret < 0) {
>> +		dev_err(common->dev, "adc clk enable failed\n");
>> +		goto err_regulator_disable;
>> +	}
>> +
>> +	common->irq = platform_get_irq(pdev, 0);
>> +	if (common->irq < 0) {
>> +		dev_err(&pdev->dev, "failed to get irq\n");
>> +		ret = common->irq;
>> +		goto err_clk_disable;
>> +	}
>> +
>> +	ret = devm_request_irq(&pdev->dev, common->irq,	stm32_adc_common_isr,
>> +			       0, pdev->name, common);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to request irq\n");
>> +		goto err_clk_disable;
>> +	}
>> +
>> +	/* Parse adc child nodes to retrieve master/slave instances data */
>> +	for_each_available_child_of_node(np, child) {
>> +		ret = stm32_adc_register(common, child);
>> +		if (ret)
>> +			goto err_unregister;
>> +	}
>> +
>> +	dev_info(&pdev->dev, "registered\n");
> No benefit in this info being provided (it's obvious, device just turned up
> in sysfs :) So drop it.
I'll fix this.
>> +
>> +	return 0;
>> +
>> +err_unregister:
>> +	list_for_each_entry(adc, &common->adc_list, adc_list)
>> +		stm32_adc_unregister(adc);
>> +
>> +err_clk_disable:
>> +	clk_disable_unprepare(common->aclk);
>> +
>> +err_regulator_disable:
>> +	regulator_disable(common->vref);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(stm32_adc_probe);
>> +
>> +int stm32_adc_remove(struct platform_device *pdev)
>> +{
>> +	struct stm32_adc_common *common = platform_get_drvdata(pdev);
>> +	struct stm32_adc *adc;
>> +
>> +	list_for_each_entry(adc, &common->adc_list, adc_list)
>> +		stm32_adc_unregister(adc);
>> +	clk_disable_unprepare(common->aclk);
>> +	regulator_disable(common->vref);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(stm32_adc_remove);
>> +
>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/iio/adc/stm32/stm32-adc.h b/drivers/iio/adc/stm32/stm32-adc.h
>> new file mode 100644
>> index 0000000..0be603c
>> --- /dev/null
>> +++ b/drivers/iio/adc/stm32/stm32-adc.h
>> @@ -0,0 +1,442 @@
>> +/*
>> + * This file is part of STM32 ADC driver
>> + *
>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
>> + *
>> + * License type: GPLv2
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>> + * See the GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef __STM32_ADC_H
>> +#define __STM32_ADC_H
>> +
>> +/*
>> + * STM32 - ADC global register map
>> + * ________________________________________________________
>> + * | Offset |                 Register                    |
>> + * --------------------------------------------------------
>> + * | 0x000  |                Master ADC1                  |
>> + * --------------------------------------------------------
>> + * | 0x100  |                Slave ADC2                   |
>> + * --------------------------------------------------------
>> + * | 0x200  |                Slave ADC3                   |
>> + * --------------------------------------------------------
>> + * | 0x300  |         Master & Slave common regs          |
>> + * --------------------------------------------------------
>> + */
>> +#define STM32_ADCX_COMN_OFFSET		0x300
>> +#define STM32_ADC_ID_MAX		3
>> +#define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>> +#define STM32_ADC_MAX_JSQ		4	/* JSQ1..JSQ4 */
>> +
>> +/* STM32 value masks */
>> +#define STM32_RESULT_MASK	GENMASK(15, 0)
>> +#define STM32_STORAGEBITS	16
>> +
>> +/* External trigger enable for regular or injected channels (exten/jexten) */
>> +enum stm32_adc_exten {
>> +	STM32_EXTEN_SWTRIG,
>> +	STM32_EXTEN_HWTRIG_RISING_EDGE,
>> +	STM32_EXTEN_HWTRIG_FALLING_EDGE,
>> +	STM32_EXTEN_HWTRIG_BOTH_EDGES,
>> +};
>> +
>> +enum stm32_adc_extsel {
>> +	STM32_EXT0,
>> +	STM32_EXT1,
>> +	STM32_EXT2,
>> +	STM32_EXT3,
>> +	STM32_EXT4,
>> +	STM32_EXT5,
>> +	STM32_EXT6,
>> +	STM32_EXT7,
>> +	STM32_EXT8,
>> +	STM32_EXT9,
>> +	STM32_EXT10,
>> +	STM32_EXT11,
>> +	STM32_EXT12,
>> +	STM32_EXT13,
>> +	STM32_EXT14,
>> +	STM32_EXT15,
>> +	STM32_EXT16,
>> +	STM32_EXT17,
>> +	STM32_EXT18,
>> +	STM32_EXT19,
>> +	STM32_EXT20,
>> +	STM32_EXT21,
>> +	STM32_EXT22,
>> +	STM32_EXT23,
>> +	STM32_EXT24,
>> +	STM32_EXT25,
>> +	STM32_EXT26,
>> +	STM32_EXT27,
>> +	STM32_EXT28,
>> +	STM32_EXT29,
>> +	STM32_EXT30,
>> +	STM32_EXT31,
>> +};
>> +
>> +enum stm32_adc_jextsel {
>> +	STM32_JEXT0,
>> +	STM32_JEXT1,
>> +	STM32_JEXT2,
>> +	STM32_JEXT3,
>> +	STM32_JEXT4,
>> +	STM32_JEXT5,
>> +	STM32_JEXT6,
>> +	STM32_JEXT7,
>> +	STM32_JEXT8,
>> +	STM32_JEXT9,
>> +	STM32_JEXT10,
>> +	STM32_JEXT11,
>> +	STM32_JEXT12,
>> +	STM32_JEXT13,
>> +	STM32_JEXT14,
>> +	STM32_JEXT15,
>> +	STM32_JEXT16,
>> +	STM32_JEXT17,
>> +	STM32_JEXT18,
>> +	STM32_JEXT19,
>> +	STM32_JEXT20,
>> +	STM32_JEXT21,
>> +	STM32_JEXT22,
>> +	STM32_JEXT23,
>> +	STM32_JEXT24,
>> +	STM32_JEXT25,
>> +	STM32_JEXT26,
>> +	STM32_JEXT27,
>> +	STM32_JEXT28,
>> +	STM32_JEXT29,
>> +	STM32_JEXT30,
>> +	STM32_JEXT31,
>> +};
>> +
>> +#define	STM32_ADC_TIMEOUT_US	100000
>> +#define	STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>> +
>> +/**
>> + * struct stm32_adc_chan_spec - specification of stm32 adc channel
>> + * @type:	IIO channel type
>> + * @channel:	channel number (single ended)
>> + * @name:	channel name (single ended)
>> + */
>> +struct stm32_adc_chan_spec {
>> +	enum iio_chan_type	type;
>> +	int			channel;
>> +	const char		*name;
>> +};
>> +
>> +/**
>> + * struct stm32_adc_trig_info - ADC trigger info
>> + * @extsel:		trigger selection for regular or injected
>> + * @name:		name of the trigger, corresponding to its source
>> + */
>> +struct stm32_adc_trig_info {
>> +	u32 extsel;
>> +	const char *name;
>> +};
>> +
>> +/**
>> + * struct stm32_adc_info - stm32 ADC, per instance config data
>> + * @name:		default name for this instance (like "adc1")
>> + * @reg:		reg offset for this instance (e.g. 0x0 for adc1...)
>> + * @channels:		Reference to stm32 channels spec
>> + * @max_channels:	Number of single ended channels
>> + */
>> +struct stm32_adc_info {
>> +	const char *name;
>> +	u32 reg;
>> +	const struct stm32_adc_chan_spec *channels;
>> +	int max_channels;
>> +};
>> +
>> +/**
>> + * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
>> + * @reg:		register offset
>> + * @mask:		bitfield mask
>> + * @shift:		left shift
>> + */
>> +struct stm32_adc_regs {
>> +	int reg;
>> +	int mask;
>> +	int shift;
>> +};
>> +
>> +/**
>> + * stm32_adc_trig_reginfo - stm32 ADC trigger control registers description
>> + * @reg:		trigger control register offset (exten/jexten)
>> + * @exten_mask:		external trigger en/polarity mask in @reg
>> + * @exten_shift:	external trigger en/polarity shift in @reg
>> + * @extsel_mask:	external trigger source mask in @reg
>> + * @extsel_shift:	external trigger source shift in @reg
>> + */
>> +struct stm32_adc_trig_reginfo {
>> +	u32 reg;
>> +	u32 exten_mask;
>> +	u32 exten_shift;
>> +	u32 extsel_mask;
>> +	u32 extsel_shift;
>> +};
>> +
>> +/**
>> + * struct stm32_adc_reginfo - stm32 ADC registers description
>> + * @isr:		interrupt status register offset
>> + * @eoc:		end of conversion mask in @isr
>> + * @jeoc:		end of injected conversion sequence mask in @isr
>> + * @ier:		interrupt enable register offset
>> + * @eocie:		end of conversion interrupt enable mask in @ier
>> + * @jeocie:		end of injected conversion sequence interrupt en mask
>> + * @dr:			data register offset
>> + * @jdr:		injected data registers offsets
>> + * @sqr_regs:		Regular sequence registers description
>> + * @jsqr_reg:		Injected sequence register description
>> + * @trig_reginfo:	regular trigger control registers description
>> + * @jtrig_reginfo:	injected trigger control registers description
>> + */
>> +struct stm32_adc_reginfo {
>> +	u32 isr;
>> +	u32 eoc;
>> +	u32 jeoc;
>> +	u32 ier;
>> +	u32 eocie;
>> +	u32 jeocie;
>> +	u32 dr;
>> +	u32 jdr[4];
>> +	const struct stm32_adc_regs *sqr_regs;
>> +	const struct stm32_adc_regs *jsqr_reg;
>> +	const struct stm32_adc_trig_reginfo *trig_reginfo;
>> +	const struct stm32_adc_trig_reginfo *jtrig_reginfo;
>> +};
>> +
>> +struct stm32_adc;
>> +
>> +/**
>> + * struct stm32_adc_ops - stm32 ADC, compatible dependent data
>> + * - stm32 ADC may work as single ADC, or as tightly coupled master/slave ADCs.
>> + *
>> + * @adc_info:		Array spec for stm32 adc master/slaves instances
>> + * @ext_triggers:	Reference to trigger info for regular channels
>> + * @jext_triggers:	Reference to trigger info for injected channels
>> + * @adc_reginfo:	stm32 ADC registers description
>> + * @highres:		Max resolution
>> + * @max_clock_rate:	Max input clock rate
>> + * @clk_sel:		routine to select common clock and prescaler
>> + * @start_conv:		routine to start conversions
>> + * @stop_conv:		routine to stop conversions
>> + * @is_started:		routine to get adc 'started' state
>> + * @regular_started	routine to check regular conversions status
>> + * @injected_started	routine to check injected conversions status
>> + * @enable:		optional routine to enable stm32 adc
>> + * @disable:		optional routine to disable stm32 adc
>> + * @is_enabled		reports enabled state
>> + */
> This is a big chunk of abstraction that seems excessive at the moment.
> I'd rather see it introduced only just before it's actually used..
> (I'm guessing it's intended for support of similar parts?)
>
> Right now it just makes the driver harder to review.
>> +struct stm32_adc_ops {
>> +	const struct stm32_adc_info *adc_info;
>> +	const struct stm32_adc_trig_info *ext_triggers;
>> +	const struct stm32_adc_trig_info *jext_triggers;
>> +	const struct stm32_adc_reginfo *adc_reginfo;
>> +	int highres;
>> +	unsigned long max_clock_rate;
>> +	int (*clk_sel)(struct stm32_adc *adc);
>> +	int (*start_conv)(struct stm32_adc *adc);
>> +	int (*stop_conv)(struct stm32_adc *adc);
>> +	bool (*is_started)(struct stm32_adc *adc);
>> +	bool (*regular_started)(struct stm32_adc *adc);
>> +	bool (*injected_started)(struct stm32_adc *adc);
>> +	int (*enable)(struct stm32_adc *adc);
>> +	void (*disable)(struct stm32_adc *adc);
>> +	bool (*is_enabled)(struct stm32_adc *adc);
>> +};
>> +
>> +struct stm32_adc_common;
>> +
>> +/**
>> + * struct stm32_adc - private data of each ADC IIO instance
>> + * @common:		reference to ADC block common data
>> + * @adc_list:		current ADC entry in common ADC list
>> + * @id:			ADC instance number (e.g. adc 1, 2 or 3)
>> + * @offset:		ADC instance register offset in ADC block
>> + * @max_channels:	Max channels number for this ADC.
>> + * @extrig_list:	External trigger list (for regular channel)
>> + * @completion:		end of single conversion completion
>> + * @buffer:		data buffer
>> + * @bufi:		data buffer index
>> + * @num_conv:		expected number of scan conversions
>> + * @injected:		use injected channels on this adc
>> + * @lock:		spinlock
>> + * @clk:		optional adc clock, for this adc instance
>> + * @calib:		optional calibration data
>> + * @en:			emulates enabled state on some stm32 adc
>> + */
>> +struct stm32_adc {
>> +	struct stm32_adc_common	*common;
>> +	struct list_head	adc_list;
>> +	int			id;
>> +	int			offset;
>> +	int			max_channels;
>> +	struct list_head	extrig_list;
>> +	struct completion	completion;
>> +	u16			*buffer;
>> +	int			bufi;
>> +	int			num_conv;
>> +	bool			injected;
>> +	spinlock_t		lock;		/* interrupt lock */
>> +	struct clk		*clk;
>> +	void			*calib;
>> +	bool			en;
>> +};
>> +
>> +/**
>> + * struct stm32_adc_common - private data of ADC driver, common to all
>> + * ADC instances (ADC block)
>> + * @dev:		device for this controller
>> + * @base:		control registers base cpu addr
>> + * @irq:		Common irq line for all adc instances
>> + * @data:		STM32 dependent data from compatible
>> + * @adc_list:		list of all stm32 ADC in this ADC block
>> + * @aclk:		common clock for the analog circuitry
>> + * @vref:		regulator reference
>> + * @vref_mv:		vref voltage (mv)
>> + * @lock:		mutex
>> + */
>> +struct stm32_adc_common {
>> +	struct device			*dev;
>> +	void __iomem			*base;
>> +	int				irq;
>> +	const struct stm32_adc_ops	*data;
>> +	struct list_head		adc_list;
>> +	struct clk			*aclk;
>> +	struct regulator		*vref;
>> +	int				vref_mv;
>> +	struct mutex			lock;	/* read_raw lock */
>> +};
>> +
>> +/* Helper routines */
>> +static inline int stm32_adc_start_conv(struct stm32_adc *adc)
>> +{
>> +	return adc->common->data->start_conv(adc);
>> +}
>> +
>> +static inline int stm32_adc_stop_conv(struct stm32_adc *adc)
>> +{
>> +	return adc->common->data->stop_conv(adc);
>> +}
>> +
>> +static inline bool stm32_adc_is_started(struct stm32_adc *adc)
>> +{
>> +	return adc->common->data->is_started(adc);
>> +}
>> +
>> +static inline bool stm32_adc_regular_started(struct stm32_adc *adc)
>> +{
>> +	return adc->common->data->regular_started(adc);
>> +}
>> +
>> +static inline bool stm32_adc_injected_started(struct stm32_adc *adc)
>> +{
>> +	return adc->common->data->injected_started(adc);
>> +}
>> +
>> +static inline bool stm32_adc_clk_sel(struct stm32_adc *adc)
>> +{
>> +	return adc->common->data->clk_sel(adc);
>> +}
>> +
>> +static inline int stm32_adc_enable(struct stm32_adc *adc)
>> +{
>> +	if (adc->common->data->enable)
>> +		return adc->common->data->enable(adc);
>> +
>> +	adc->en = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static inline bool stm32_adc_is_enabled(struct stm32_adc *adc)
>> +{
>> +	if (adc->common->data->is_enabled)
>> +		return adc->common->data->is_enabled(adc);
>> +	else
>> +		return adc->en;
>> +}
>> +
>> +static inline void stm32_adc_disable(struct stm32_adc *adc)
>> +{
>> +	/* Check there is no regular or injected on-going conversions */
>> +	if (stm32_adc_is_started(adc))
>> +		return;
>> +
>> +	if (adc->common->data->disable)
>> +		adc->common->data->disable(adc);
>> +	else
>> +		adc->en = false;
>> +}
>> +
>> +/* STM32 ADC registers access routines */
>> +static inline u32 stm32_adc_common_readl(struct stm32_adc_common *com, u32 reg)
>> +{
>> +	u32 val = readl_relaxed(com->base + reg);
>> +
>> +	return val;
>> +}
>> +
>> +static inline void stm32_adc_common_writel(struct stm32_adc_common *com,
>> +					   u32 reg, u32 val)
>> +{
>> +	writel_relaxed(val, com->base + reg);
>> +}
>> +
>> +static inline u32 stm32_adc_readl(struct stm32_adc *adc, u32 reg)
>> +{
>> +	u32 val = readl_relaxed(adc->common->base + adc->offset + reg);
>> +
>> +	return val;
>> +}
>> +
>> +#define stm32_adc_readl_addr(addr)	stm32_adc_readl(adc, addr)
>> +
>> +#define stm32_adc_readl_poll_timeout(reg, val, cond, sleep_us, timeout_us) \
>> +	readx_poll_timeout(stm32_adc_readl_addr, reg, val, \
>> +			   cond, sleep_us, timeout_us)
>> +
>> +static inline void stm32_adc_writel(struct stm32_adc *adc, u32 reg, u32 val)
>> +{
>> +	writel_relaxed(val, adc->common->base + adc->offset + reg);
>> +}
>> +
>> +static inline void stm32_adc_set_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>> +{
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&adc->lock, flags);
>> +	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) | bits);
>> +	spin_unlock_irqrestore(&adc->lock, flags);
>> +}
>> +
>> +static inline void stm32_adc_clr_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>> +{
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&adc->lock, flags);
>> +	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) & ~bits);
>> +	spin_unlock_irqrestore(&adc->lock, flags);
>> +}
>> +
>> +/* STM32 common extended attributes */
>> +extern const struct iio_enum stm32_adc_trig_pol;
>> +int stm32_adc_probe(struct platform_device *pdev);
>> +int stm32_adc_remove(struct platform_device *pdev);
>> +
>> +#endif
>> diff --git a/drivers/iio/adc/stm32/stm32f4-adc.c b/drivers/iio/adc/stm32/stm32f4-adc.c
>> new file mode 100644
>> index 0000000..147fe9c
>> --- /dev/null
>> +++ b/drivers/iio/adc/stm32/stm32f4-adc.c
>> @@ -0,0 +1,574 @@
>> +/*
>> + * This file is part of STM32F4 ADC driver
>> + *
>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
>> + *
>> + * License type: GPLv2
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>> + * See the GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/platform_device.h>
>> +#include "stm32-adc.h"
>> +
>> +/*
>> + * STM32F4 - ADC global register map
>> + * ________________________________________________________
>> + * | Offset |                 Register                    |
>> + * --------------------------------------------------------
>> + * | 0x000  |                Master ADC1                  |
>> + * --------------------------------------------------------
>> + * | 0x100  |                Slave ADC2                   |
>> + * --------------------------------------------------------
>> + * | 0x200  |                Slave ADC3                   |
>> + * --------------------------------------------------------
>> + * | 0x300  |         Master & Slave common regs          |
>> + * --------------------------------------------------------
>> + */
>> +
>> +/* STM32F4 - Registers for each ADC instance */
>> +#define STM32F4_ADCX_SR			0x00
>> +#define STM32F4_ADCX_CR1		0x04
>> +#define STM32F4_ADCX_CR2		0x08
>> +#define STM32F4_ADCX_SMPR1		0x0C
>> +#define STM32F4_ADCX_SMPR2		0x10
>> +#define STM32F4_ADCX_HTR		0x24
>> +#define STM32F4_ADCX_LTR		0x28
>> +#define STM32F4_ADCX_SQR1		0x2C
>> +#define STM32F4_ADCX_SQR2		0x30
>> +#define STM32F4_ADCX_SQR3		0x34
>> +#define STM32F4_ADCX_JSQR		0x38
>> +#define STM32F4_ADCX_JDR1		0x3C
>> +#define STM32F4_ADCX_JDR2		0x40
>> +#define STM32F4_ADCX_JDR3		0x44
>> +#define STM32F4_ADCX_JDR4		0x48
>> +#define STM32F4_ADCX_DR			0x4C
>> +
>> +/* STM32 - Master & slave registers (common for all instances: 1, 2 & 3) */
>> +#define STM32F4_ADC_CSR			(STM32_ADCX_COMN_OFFSET + 0x00)
>> +#define STM32F4_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x04)
>> +#define STM32F4_ADC_CDR			(STM32_ADCX_COMN_OFFSET + 0x08)
>> +
>> +/* STM32F4_ADCX_SR - bit fields */
>> +#define STM32F4_OVR			BIT(5)
>> +#define STM32F4_STRT			BIT(4)
>> +#define STM32F4_JSTRT			BIT(3)
>> +#define STM32F4_JEOC			BIT(2)
>> +#define STM32F4_EOC			BIT(1)
>> +#define STM32F4_AWD			BIT(0)
>> +
>> +/* STM32F4_ADCX_CR1 - bit fields */
>> +#define STM32F4_OVRIE			BIT(26)
>> +#define STM32F4_RES_SHIFT		24
>> +#define STM32F4_RES_MASK		GENMASK(25, 24)
>> +#define STM32F4_AWDEN			BIT(23)
>> +#define STM32F4_JAWDEN			BIT(22)
>> +#define STM32F4_DISCNUM_SHIFT		13
>> +#define STM32F4_DISCNUM_MASK		GENMASK(15, 13)
>> +#define STM32F4_JDISCEN			BIT(12)
>> +#define STM32F4_DISCEN			BIT(11)
>> +#define STM32F4_JAUTO			BIT(10)
>> +#define STM32F4_AWDSGL			BIT(9)
>> +#define STM32F4_SCAN			BIT(8)
>> +#define STM32F4_JEOCIE			BIT(7)
>> +#define STM32F4_AWDIE			BIT(6)
>> +#define STM32F4_EOCIE			BIT(5)
>> +#define STM32F4_AWDCH_SHIFT		0
>> +#define STM32F4_AWDCH_MASK		GENMASK(4, 0)
>> +
>> +/* STM32F4_ADCX_CR2 - bit fields */
>> +#define STM32F4_SWSTART			BIT(30)
>> +#define STM32F4_EXTEN_SHIFT		28
>> +#define STM32F4_EXTEN_MASK		GENMASK(29, 28)
>> +#define STM32F4_EXTSEL_SHIFT		24
>> +#define STM32F4_EXTSEL_MASK		GENMASK(27, 24)
>> +#define STM32F4_JSWSTART		BIT(22)
>> +#define STM32F4_JEXTEN_SHIFT		20
>> +#define STM32F4_JEXTEN_MASK		GENMASK(21, 20)
>> +#define STM32F4_JEXTSEL_SHIFT		16
>> +#define STM32F4_JEXTSEL_MASK		GENMASK(19, 16)
>> +#define STM32F4_ALIGN			BIT(11)
>> +#define STM32F4_EOCS			BIT(10)
>> +#define STM32F4_DDS			BIT(9)
>> +#define STM32F4_DMA			BIT(8)
>> +#define STM32F4_CONT			BIT(1)
>> +#define STM32F4_ADON			BIT(0)
>> +
>> +/* STM32F4_ADCX_SMPR1 - bit fields */
>> +#define STM32F4_SMP18_SHIFT		24
>> +#define STM32F4_SMP18_MASK		GENMASK(26, 24)
>> +#define STM32F4_SMP17_SHIFT		21
>> +#define STM32F4_SMP17_MASK		GENMASK(23, 21)
>> +#define STM32F4_SMP16_SHIFT		18
>> +#define STM32F4_SMP16_MASK		GENMASK(20, 18)
>> +#define STM32F4_SMP15_SHIFT		15
>> +#define STM32F4_SMP15_MASK		GENMASK(17, 15)
>> +#define STM32F4_SMP14_SHIFT		12
>> +#define STM32F4_SMP14_MASK		GENMASK(14, 12)
>> +#define STM32F4_SMP13_SHIFT		9
>> +#define STM32F4_SMP13_MASK		GENMASK(11, 9)
>> +#define STM32F4_SMP12_SHIFT		6
>> +#define STM32F4_SMP12_MASK		GENMASK(8, 6)
>> +#define STM32F4_SMP11_SHIFT		3
>> +#define STM32F4_SMP11_MASK		GENMASK(5, 3)
>> +#define STM32F4_SMP10_SHIFT		0
>> +#define STM32F4_SMP10_MASK		GENMASK(2, 0)
>> +
>> +/* STM32F4_ADCX_SMPR2 - bit fields */
>> +#define STM32F4_SMP9_SHIFT		27
>> +#define STM32F4_SMP9_MASK		GENMASK(29, 27)
>> +#define STM32F4_SMP8_SHIFT		24
>> +#define STM32F4_SMP8_MASK		GENMASK(26, 24)
>> +#define STM32F4_SMP7_SHIFT		21
>> +#define STM32F4_SMP7_MASK		GENMASK(23, 21)
>> +#define STM32F4_SMP6_SHIFT		18
>> +#define STM32F4_SMP6_MASK		GENMASK(20, 18)
>> +#define STM32F4_SMP5_SHIFT		15
>> +#define STM32F4_SMP5_MASK		GENMASK(17, 15)
>> +#define STM32F4_SMP4_SHIFT		12
>> +#define STM32F4_SMP4_MASK		GENMASK(14, 12)
>> +#define STM32F4_SMP3_SHIFT		9
>> +#define STM32F4_SMP3_MASK		GENMASK(11, 9)
>> +#define STM32F4_SMP2_SHIFT		6
>> +#define STM32F4_SMP2_MASK		GENMASK(8, 6)
>> +#define STM32F4_SMP1_SHIFT		3
>> +#define STM32F4_SMP1_MASK		GENMASK(5, 3)
>> +#define STM32F4_SMP0_SHIFT		0
>> +#define STM32F4_SMP0_MASK		GENMASK(2, 0)
>> +enum stm32f4_adc_smpr {
>> +	STM32F4_SMPR_3_CK_CYCLES,
>> +	STM32F4_SMPR_15_CK_CYCLES,
>> +	STM32F4_SMPR_28_CK_CYCLES,
>> +	STM32F4_SMPR_56_CK_CYCLES,
>> +	STM32F4_SMPR_84_CK_CYCLES,
>> +	STM32F4_SMPR_112_CK_CYCLES,
>> +	STM32F4_SMPR_144_CK_CYCLES,
>> +	STM32F4_SMPR_480_CK_CYCLES,
>> +};
>> +
>> +/* STM32F4_ADCX_SQR1 - bit fields */
>> +#define STM32F4_L_SHIFT			20
>> +#define STM32F4_L_MASK			GENMASK(23, 20)
>> +#define STM32F4_SQ16_SHIFT		15
>> +#define STM32F4_SQ16_MASK		GENMASK(19, 15)
>> +#define STM32F4_SQ15_SHIFT		10
>> +#define STM32F4_SQ15_MASK		GENMASK(14, 10)
>> +#define STM32F4_SQ14_SHIFT		5
>> +#define STM32F4_SQ14_MASK		GENMASK(9, 5)
>> +#define STM32F4_SQ13_SHIFT		0
>> +#define STM32F4_SQ13_MASK		GENMASK(4, 0)
>> +
>> +/* STM32F4_ADCX_SQR2 - bit fields */
>> +#define STM32F4_SQ12_SHIFT		25
>> +#define STM32F4_SQ12_MASK		GENMASK(29, 25)
>> +#define STM32F4_SQ11_SHIFT		20
>> +#define STM32F4_SQ11_MASK		GENMASK(24, 20)
>> +#define STM32F4_SQ10_SHIFT		15
>> +#define STM32F4_SQ10_MASK		GENMASK(19, 15)
>> +#define STM32F4_SQ9_SHIFT		10
>> +#define STM32F4_SQ9_MASK		GENMASK(14, 10)
>> +#define STM32F4_SQ8_SHIFT		5
>> +#define STM32F4_SQ8_MASK		GENMASK(9, 5)
>> +#define STM32F4_SQ7_SHIFT		0
>> +#define STM32F4_SQ7_MASK		GENMASK(4, 0)
>> +
>> +/* STM32F4_ADCX_SQR3 - bit fields */
>> +#define STM32F4_SQ6_SHIFT		25
>> +#define STM32F4_SQ6_MASK		GENMASK(29, 25)
>> +#define STM32F4_SQ5_SHIFT		20
>> +#define STM32F4_SQ5_MASK		GENMASK(24, 20)
>> +#define STM32F4_SQ4_SHIFT		15
>> +#define STM32F4_SQ4_MASK		GENMASK(19, 15)
>> +#define STM32F4_SQ3_SHIFT		10
>> +#define STM32F4_SQ3_MASK		GENMASK(14, 10)
>> +#define STM32F4_SQ2_SHIFT		5
>> +#define STM32F4_SQ2_MASK		GENMASK(9, 5)
>> +#define STM32F4_SQ1_SHIFT		0
>> +#define STM32F4_SQ1_MASK		GENMASK(4, 0)
>> +
>> +/* STM32F4_ADCX_JSQR - bit fields */
>> +#define STM32F4_JL_SHIFT		20
>> +#define STM32F4_JL_MASK			GENMASK(21, 20)
>> +#define STM32F4_JSQ4_SHIFT		15
>> +#define STM32F4_JSQ4_MASK		GENMASK(19, 15)
>> +#define STM32F4_JSQ3_SHIFT		10
>> +#define STM32F4_JSQ3_MASK		GENMASK(14, 10)
>> +#define STM32F4_JSQ2_SHIFT		5
>> +#define STM32F4_JSQ2_MASK		GENMASK(9, 5)
>> +#define STM32F4_JSQ1_SHIFT		0
>> +#define STM32F4_JSQ1_MASK		GENMASK(4, 0)
>> +
>> +/* STM32F4_ADC_CCR - bit fields */
>> +#define STM32F4_ADC_ADCPRE_SHIFT	16
>> +#define STM32F4_ADC_ADCPRE_MASK		GENMASK(17, 16)
>> +
>> +/*
>> + * stm32 ADC1, ADC2 & ADC3 are tightly coupled and may be used in multi mode
>> + * Define here all inputs for all ADC instances
>> + */
>> +static const struct stm32_adc_chan_spec stm32f4_adc1_channels[] = {
>> +	/* master ADC1 */
>> +	{ IIO_VOLTAGE, 0, "in0" },
>> +	{ IIO_VOLTAGE, 1, "in1" },
>> +	{ IIO_VOLTAGE, 2, "in2" },
>> +	{ IIO_VOLTAGE, 3, "in3" },
>> +	{ IIO_VOLTAGE, 4, "in4" },
>> +	{ IIO_VOLTAGE, 5, "in5" },
>> +	{ IIO_VOLTAGE, 6, "in6" },
>> +	{ IIO_VOLTAGE, 7, "in7" },
>> +	{ IIO_VOLTAGE, 8, "in8" },
>> +	{ IIO_VOLTAGE, 9, "in9" },
>> +	{ IIO_VOLTAGE, 10, "in10" },
>> +	{ IIO_VOLTAGE, 11, "in11" },
>> +	{ IIO_VOLTAGE, 12, "in12" },
>> +	{ IIO_VOLTAGE, 13, "in13" },
>> +	{ IIO_VOLTAGE, 14, "in14" },
>> +	{ IIO_VOLTAGE, 15, "in15" },
>> +	/* internal analog sources available on input 16 to 18 */
>> +	{ IIO_VOLTAGE, 16, "in16" },
>> +	{ IIO_VOLTAGE, 17, "in17" },
>> +	{ IIO_VOLTAGE, 18, "in18" },
>> +};
>> +
>> +static const struct stm32_adc_chan_spec stm32f4_adc23_channels[] = {
>> +	/* slave ADC2 /	ADC3 */
>> +	{ IIO_VOLTAGE, 0, "in0" },
>> +	{ IIO_VOLTAGE, 1, "in1" },
>> +	{ IIO_VOLTAGE, 2, "in2" },
>> +	{ IIO_VOLTAGE, 3, "in3" },
>> +	{ IIO_VOLTAGE, 4, "in4" },
>> +	{ IIO_VOLTAGE, 5, "in5" },
>> +	{ IIO_VOLTAGE, 6, "in6" },
>> +	{ IIO_VOLTAGE, 7, "in7" },
>> +	{ IIO_VOLTAGE, 8, "in8" },
>> +	{ IIO_VOLTAGE, 9, "in9" },
>> +	{ IIO_VOLTAGE, 10, "in10" },
>> +	{ IIO_VOLTAGE, 11, "in11" },
>> +	{ IIO_VOLTAGE, 12, "in12" },
>> +	{ IIO_VOLTAGE, 13, "in13" },
>> +	{ IIO_VOLTAGE, 14, "in14" },
>> +	{ IIO_VOLTAGE, 15, "in15" },
>> +};
>> +
>> +/* Triggers for regular channels */
>> +static const struct stm32_adc_trig_info stm32f4_adc_ext_triggers[] = {
>> +	{ STM32_EXT0, "TIM1_CH1" },
>> +	{ STM32_EXT1, "TIM1_CH2" },
>> +	{ STM32_EXT2, "TIM1_CH3" },
>> +	{ STM32_EXT3, "TIM2_CH2" },
>> +	{ STM32_EXT4, "TIM2_CH3" },
>> +	{ STM32_EXT5, "TIM2_CH4" },
>> +	{ STM32_EXT6, "TIM2_TRGO" },
>> +	{ STM32_EXT7, "TIM3_CH1" },
>> +	{ STM32_EXT8, "TIM3_TRGO" },
>> +	{ STM32_EXT9, "TIM4_CH4" },
>> +	{ STM32_EXT10, "TIM5_CH1" },
>> +	{ STM32_EXT11, "TIM5_CH2" },
>> +	{ STM32_EXT12, "TIM5_CH3" },
>> +	{ STM32_EXT13, "TIM8_CH1" },
>> +	{ STM32_EXT14, "TIM8_TRGO" },
>> +	{ STM32_EXT15, "EXTI_11" },
>> +	{},
>> +};
>> +
>> +/* Triggers for injected channels */
>> +static const struct stm32_adc_trig_info  stm32f4_adc_jext_triggers[] = {
>> +	{ STM32_JEXT0, "TIM1_CH4" },
>> +	{ STM32_JEXT1, "TIM1_TRGO" },
>> +	{ STM32_JEXT2, "TIM2_CH1" },
>> +	{ STM32_JEXT3, "TIM2_TRGO" },
>> +	{ STM32_JEXT4, "TIM3_CH2" },
>> +	{ STM32_JEXT5, "TIM3_CH4" },
>> +	{ STM32_JEXT6, "TIM4_CH1" },
>> +	{ STM32_JEXT7, "TIM4_CH2" },
>> +	{ STM32_JEXT8, "TIM4_CH3" },
>> +	{ STM32_JEXT9, "TIM4_TRGO" },
>> +	{ STM32_JEXT10, "TIM5_CH4" },
>> +	{ STM32_JEXT11, "TIM5_TRGO" },
>> +	{ STM32_JEXT12, "TIM8_CH2" },
>> +	{ STM32_JEXT13, "TIM8_CH3" },
>> +	{ STM32_JEXT14, "TIM8_CH4" },
>> +	{ STM32_JEXT15, "EXTI_15" },
>> +	{},
>> +};
>> +
>> +static const struct stm32_adc_info stm32f4_adc_info[] = {
>> +	{
>> +		.name = "adc1-master",
>> +		.reg = 0x0,
>> +		.channels = stm32f4_adc1_channels,
>> +		.max_channels = ARRAY_SIZE(stm32f4_adc1_channels),
>> +	},
>> +	{
>> +		.name = "adc2-slave",
>> +		.reg = 0x100,
>> +		.channels = stm32f4_adc23_channels,
>> +		.max_channels = ARRAY_SIZE(stm32f4_adc23_channels),
>> +	},
>> +	{
>> +		.name = "adc3-slave",
>> +		.reg = 0x200,
>> +		.channels = stm32f4_adc23_channels,
>> +		.max_channels = ARRAY_SIZE(stm32f4_adc23_channels),
>> +	},
>> +	{},
>> +};
>> +
>> +/**
>> + * stm32f4_sqr_regs - describe regular sequence registers
>> + * - L: sequence len (register & bit field)
>> + * - SQ1..SQ16: sequence entries (register & bit field)
>> + */
>> +static const struct stm32_adc_regs stm32f4_sqr_regs[STM32_ADC_MAX_SQ + 1] = {
>> +	/* L: len bit field description to be kept as first element */
>> +	{ STM32F4_ADCX_SQR1, STM32F4_L_MASK, STM32F4_L_SHIFT },
>> +	/* SQ1..SQ16 registers & bit fields */
>> +	{ STM32F4_ADCX_SQR3, STM32F4_SQ1_MASK, STM32F4_SQ1_SHIFT },
>> +	{ STM32F4_ADCX_SQR3, STM32F4_SQ2_MASK, STM32F4_SQ2_SHIFT },
>> +	{ STM32F4_ADCX_SQR3, STM32F4_SQ3_MASK, STM32F4_SQ3_SHIFT },
>> +	{ STM32F4_ADCX_SQR3, STM32F4_SQ4_MASK, STM32F4_SQ4_SHIFT },
>> +	{ STM32F4_ADCX_SQR3, STM32F4_SQ5_MASK, STM32F4_SQ5_SHIFT },
>> +	{ STM32F4_ADCX_SQR3, STM32F4_SQ6_MASK, STM32F4_SQ6_SHIFT },
>> +	{ STM32F4_ADCX_SQR2, STM32F4_SQ7_MASK, STM32F4_SQ7_SHIFT },
>> +	{ STM32F4_ADCX_SQR2, STM32F4_SQ8_MASK, STM32F4_SQ8_SHIFT },
>> +	{ STM32F4_ADCX_SQR2, STM32F4_SQ9_MASK, STM32F4_SQ9_SHIFT },
>> +	{ STM32F4_ADCX_SQR2, STM32F4_SQ10_MASK, STM32F4_SQ10_SHIFT },
>> +	{ STM32F4_ADCX_SQR2, STM32F4_SQ11_MASK, STM32F4_SQ11_SHIFT },
>> +	{ STM32F4_ADCX_SQR2, STM32F4_SQ12_MASK, STM32F4_SQ12_SHIFT },
>> +	{ STM32F4_ADCX_SQR1, STM32F4_SQ13_MASK, STM32F4_SQ13_SHIFT },
>> +	{ STM32F4_ADCX_SQR1, STM32F4_SQ14_MASK, STM32F4_SQ14_SHIFT },
>> +	{ STM32F4_ADCX_SQR1, STM32F4_SQ15_MASK, STM32F4_SQ15_SHIFT },
>> +	{ STM32F4_ADCX_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>> +};
>> +
>> +/**
>> + * stm32f4_jsqr_reg - describe injected sequence register:
>> + * - JL: injected sequence len
>> + * - JSQ4..SQ1: sequence entries
>> + * When JL == 3, ADC converts JSQ1, JSQ2, JSQ3, JSQ4
>> + * When JL == 2, ADC converts JSQ2, JSQ3, JSQ4
>> + * When JL == 1, ADC converts JSQ3, JSQ4
>> + * When JL == 0, ADC converts JSQ4
>> + */
>> +static const struct stm32_adc_regs stm32f4_jsqr_reg[STM32_ADC_MAX_JSQ + 1] = {
>> +	/* JL: len bit field description to be kept as first element */
>> +	{STM32F4_ADCX_JSQR, STM32F4_JL_MASK, STM32F4_JL_SHIFT},
>> +	/* JSQ4..JSQ1 registers & bit fields */
>> +	{STM32F4_ADCX_JSQR, STM32F4_JSQ4_MASK, STM32F4_JSQ4_SHIFT},
>> +	{STM32F4_ADCX_JSQR, STM32F4_JSQ3_MASK, STM32F4_JSQ3_SHIFT},
>> +	{STM32F4_ADCX_JSQR, STM32F4_JSQ2_MASK, STM32F4_JSQ2_SHIFT},
>> +	{STM32F4_ADCX_JSQR, STM32F4_JSQ1_MASK, STM32F4_JSQ1_SHIFT},
>> +};
>> +
>> +static const struct stm32_adc_trig_reginfo stm32f4_adc_trig_reginfo = {
>> +	.reg = STM32F4_ADCX_CR2,
>> +	.exten_mask = STM32F4_EXTEN_MASK,
>> +	.exten_shift = STM32F4_EXTEN_SHIFT,
>> +	.extsel_mask = STM32F4_EXTSEL_MASK,
>> +	.extsel_shift = STM32F4_EXTSEL_SHIFT,
>> +};
>> +
>> +static const struct stm32_adc_trig_reginfo stm32f4_adc_jtrig_reginfo = {
>> +	.reg = STM32F4_ADCX_CR2,
>> +	.exten_mask = STM32F4_JEXTEN_MASK,
>> +	.exten_shift = STM32F4_JEXTEN_SHIFT,
>> +	.extsel_mask = STM32F4_JEXTSEL_MASK,
>> +	.extsel_shift = STM32F4_JEXTSEL_SHIFT,
>> +};
>> +
>> +static const struct stm32_adc_reginfo stm32f4_adc_reginfo = {
>> +	.isr = STM32F4_ADCX_SR,
>> +	.eoc = STM32F4_EOC,
>> +	.jeoc = STM32F4_JEOC,
>> +	.ier = STM32F4_ADCX_CR1,
>> +	.eocie = STM32F4_EOCIE,
>> +	.jeocie = STM32F4_JEOCIE,
>> +	.dr = STM32F4_ADCX_DR,
>> +	.jdr = {
>> +		STM32F4_ADCX_JDR1,
>> +		STM32F4_ADCX_JDR2,
>> +		STM32F4_ADCX_JDR3,
>> +		STM32F4_ADCX_JDR4,
>> +	},
>> +	.sqr_regs = stm32f4_sqr_regs,
>> +	.jsqr_reg = stm32f4_jsqr_reg,
>> +	.trig_reginfo = &stm32f4_adc_trig_reginfo,
>> +	.jtrig_reginfo = &stm32f4_adc_jtrig_reginfo,
>> +};
>> +
>> +static bool stm32f4_adc_is_started(struct stm32_adc *adc)
>> +{
>> +	u32 val = stm32_adc_readl(adc, STM32F4_ADCX_CR2) & STM32F4_ADON;
>> +
>> +	return !!val;
>> +}
>> +
>> +static bool stm32f4_adc_regular_started(struct stm32_adc *adc)
>> +{
>> +	u32 val = stm32_adc_readl(adc, STM32F4_ADCX_SR) & STM32F4_STRT;
>> +
>> +	return !!val;
>> +}
>> +
>> +static bool stm32f4_adc_injected_started(struct stm32_adc *adc)
>> +{
>> +	u32 val = stm32_adc_readl(adc, STM32F4_ADCX_SR) & STM32F4_JSTRT;
>> +
>> +	return !!val;
>> +}
>> +
>> +/**
>> + * stm32f4_adc_start_conv() - Start regular or injected conversions
>> + * @adc: stm32 adc instance
>> + *
>> + * Start single conversions for regular or injected channels.
>> + */
>> +static int stm32f4_adc_start_conv(struct stm32_adc *adc)
>> +{
>> +	u32 trig_msk, start_msk;
>> +
>> +	dev_dbg(adc->common->dev, "%s %s\n", __func__,
>> +		adc->injected ? "injected" : "regular");
>> +
>> +	stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>> +
>> +	if (!stm32f4_adc_is_started(adc)) {
>> +		stm32_adc_set_bits(adc, STM32F4_ADCX_CR2,
>> +				   STM32F4_EOCS | STM32F4_ADON);
>> +
>> +		/* Wait for Power-up time (tSTAB from datasheet) */
>> +		usleep_range(2, 3);
>> +	}
>> +
>> +	if (adc->injected) {
>> +		trig_msk = STM32F4_JEXTEN_MASK;
>> +		start_msk = STM32F4_JSWSTART;
>> +	} else {
>> +		trig_msk = STM32F4_EXTEN_MASK;
>> +		start_msk = STM32F4_SWSTART;
>> +	}
>> +
>> +	/* Software start ? (e.g. trigger detection disabled ?) */
>> +	if (!(stm32_adc_readl(adc, STM32F4_ADCX_CR2) & trig_msk))
>> +		stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, start_msk);
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32f4_adc_stop_conv(struct stm32_adc *adc)
>> +{
>> +	u32 val;
>> +
>> +	dev_dbg(adc->common->dev, "%s %s\n", __func__,
>> +		adc->injected ? "injected" : "regular");
>> +
>> +	/* First disable trigger for either regular or injected channels */
>> +	if (adc->injected) {
>> +		stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_JEXTEN_MASK);
>> +		stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_JSTRT);
>> +	} else {
>> +		stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
>> +		stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_STRT);
>> +	}
>> +
>> +	/* Disable adc when all triggered conversion have been disabled */
>> +	val = stm32_adc_readl(adc, STM32F4_ADCX_CR2);
>> +	val &= STM32F4_EXTEN_MASK | STM32F4_JEXTEN_MASK;
>> +	if (!val) {
>> +		stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>> +		stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_ADON);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/* ADC internal common clock prescaler division ratios */
>> +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
>> +
>> +/**
>> + * stm32f4_adc_clk_sel() - Select ADC common clock prescaler
>> + * @adc: stm32 adc instance
>> + * Select clock prescaler used for analog conversions.
>> + */
>> +static int stm32f4_adc_clk_sel(struct stm32_adc *adc)
>> +{
>> +	struct stm32_adc_common *common = adc->common;
>> +	unsigned long rate;
>> +	u32 val;
>> +	int i;
>> +
>> +	/* Common prescaler is set only once, when 1st ADC instance starts */
>> +	list_for_each_entry(adc, &common->adc_list, adc_list)
>> +		if (stm32f4_adc_is_started(adc))
>> +			return 0;
>> +
>> +	rate = clk_get_rate(common->aclk);
>> +	for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
>> +		if ((rate / stm32f4_pclk_div[i]) <=
>> +		    common->data->max_clock_rate)
>> +			break;
>> +	}
>> +	if (i >= ARRAY_SIZE(stm32f4_pclk_div))
>> +		return -EINVAL;
>> +
>> +	val = stm32_adc_common_readl(common, STM32F4_ADC_CCR);
>> +	val &= ~STM32F4_ADC_ADCPRE_MASK;
>> +	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
>> +	stm32_adc_common_writel(common, STM32F4_ADC_CCR, val);
>> +
>> +	dev_dbg(common->dev, "Using analog clock source at %ld kHz\n",
>> +		rate / (stm32f4_pclk_div[i] * 1000));
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct stm32_adc_ops stm32f4_adc_ops = {
>> +	.adc_info = stm32f4_adc_info,
>> +	.ext_triggers = stm32f4_adc_ext_triggers,
>> +	.jext_triggers = stm32f4_adc_jext_triggers,
>> +	.adc_reginfo = &stm32f4_adc_reginfo,
>> +	.highres = 12,
>> +	.max_clock_rate = 36000000,
>> +	.clk_sel = stm32f4_adc_clk_sel,
>> +	.start_conv = stm32f4_adc_start_conv,
>> +	.stop_conv = stm32f4_adc_stop_conv,
>> +	.is_started = stm32f4_adc_is_started,
>> +	.regular_started = stm32f4_adc_regular_started,
>> +	.injected_started = stm32f4_adc_injected_started,
>> +};
>> +
>> +static const struct of_device_id stm32f4_adc_of_match[] = {
>> +	{ .compatible = "st,stm32f4-adc", .data = (void *)&stm32f4_adc_ops},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32f4_adc_of_match);
>> +
>> +static struct platform_driver stm32f4_adc_driver = {
>> +	.probe = stm32_adc_probe,
>> +	.remove = stm32_adc_remove,
>> +	.driver = {
>> +		.name = "stm32f4-adc",
>> +		.of_match_table = stm32f4_adc_of_match,
>> +	},
>> +};
>> +
>> +module_platform_driver(stm32f4_adc_driver);
>> +
>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics STM32F4 ADC driver");
>> +MODULE_LICENSE("GPL v2");
>>

^ permalink raw reply

* [PATCH 09/13] mmc: dw_mmc: remove the "clock-freq-min-max" property
From: Heiko Stübner @ 2016-11-03  8:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103062135.10697-10-jh80.chung@samsung.com>

Hi Jaehoon,

Am Donnerstag, 3. November 2016, 15:21:31 schrieb Jaehoon Chung:
> Remove the "clock-freq-min-max" property.
> There is "max-frequency" property in drivers/mmc/core/host.c
> It can be used for getting maximum frequency.
> 
> And minimum clock value is assigned to 100K by default.
> Because MMC core should check the initial clock value from
> 400K to 100K until finding correct value.
> It's why Minimum value puts 100K by default.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt |  3 ---
>  drivers/mmc/host/dw_mmc.c                                  | 13
> ++++--------- 2 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt index
> 4e00e85..21c8251 100644
> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> @@ -56,9 +56,6 @@ Optional properties:
>    is specified and the ciu clock is specified then we'll try to set the ciu
> clock to this at probe time.
> 
> -* clock-freq-min-max: Minimum and Maximum clock frequency for card output
> -  clock(cclk_out). If it's not specified, max is 200MHZ and min is 400KHz
> by default. -

I'd think devicetree people will protest about simply removing this property 
without deprecating it first - to be backwards compatible with old devicetrees.

Especially as this property was added already back in 2013, so has been in use 
for quite some time.


Heiko

^ permalink raw reply

* [PATCH v6 4/4] arm64: dts: add Pine64 support
From: Maxime Ripard @ 2016-11-03  8:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <83b8ff39-91f0-4e5c-61a5-e21f31c909ff@arm.com>

On Wed, Nov 02, 2016 at 10:05:09PM +0000, Andr? Przywara wrote:
> On 02/11/16 21:50, Maxime Ripard wrote:
> > From: Andre Przywara <andre.przywara@arm.com>
> > 
> > The Pine64 is a cost-efficient development board based on the
> > Allwinner A64 SoC.
> > There are three models: the basic version with Fast Ethernet and
> > 512 MB of DRAM (Pine64) and two Pine64+ versions, which both
> > feature Gigabit Ethernet and additional connectors for touchscreens
> > and a camera. Or as my son put it: "Those are smaller and these are
> > missing." ;-)
> > The two Pine64+ models just differ in the amount of DRAM
> > (1GB vs. 2GB). Since U-Boot will figure out the right size for us and
> > patches the DT accordingly we just need to provide one DT for the
> > Pine64+.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > [Maxime: Removed the common DTSI and include directly the pine64 DTS]
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  arch/arm64/boot/dts/Makefile                             |  1 +-
> >  arch/arm64/boot/dts/allwinner/Makefile                   |  5 +-
> >  arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts | 50 ++++++-
> >  arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts      | 74 +++++++++-
> >  4 files changed, 130 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/arm64/boot/dts/allwinner/Makefile
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > 
> > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> > index 6684f97c2722..080232b0270e 100644
> > --- a/arch/arm64/boot/dts/Makefile
> > +++ b/arch/arm64/boot/dts/Makefile
> > @@ -1,4 +1,5 @@
> >  dts-dirs += al
> > +dts-dirs += allwinner
> >  dts-dirs += altera
> >  dts-dirs += amd
> >  dts-dirs += amlogic
> > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > new file mode 100644
> > index 000000000000..1e29a5ae8282
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > @@ -0,0 +1,5 @@
> > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-pine64.dtb
> > +
> > +always		:= $(dtb-y)
> > +subdir-y	:= $(dts-dirs)
> > +clean-files	:= *.dtb
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
> > new file mode 100644
> > index 000000000000..790d14daaa6a
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
> > @@ -0,0 +1,50 @@
> > +/*
> > + * Copyright (c) 2016 ARM Ltd.
> > + *
> > + * This file is dual-licensed: you can use it either under the terms
> > + * of the GPL or the X11 license, at your option. Note that this dual
> > + * licensing only applies to this file, and not this project as a
> > + * whole.
> > + *
> > + *  a) This library is free software; you can redistribute it and/or
> > + *     modify it under the terms of the GNU General Public License as
> > + *     published by the Free Software Foundation; either version 2 of the
> > + *     License, or (at your option) any later version.
> > + *
> > + *     This library is distributed in the hope that it will be useful,
> > + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *     GNU General Public License for more details.
> > + *
> > + * Or, alternatively,
> > + *
> > + *  b) Permission is hereby granted, free of charge, to any person
> > + *     obtaining a copy of this software and associated documentation
> > + *     files (the "Software"), to deal in the Software without
> > + *     restriction, including without limitation the rights to use,
> > + *     copy, modify, merge, publish, distribute, sublicense, and/or
> > + *     sell copies of the Software, and to permit persons to whom the
> > + *     Software is furnished to do so, subject to the following
> > + *     conditions:
> > + *
> > + *     The above copyright notice and this permission notice shall be
> > + *     included in all copies or substantial portions of the Software.
> > + *
> > + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> > + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> > + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> > + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + *     OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#include "sun50i-a64-pine64.dts"
> > +
> > +/ {
> > +	model = "Pine64+";
> > +	compatible = "pine64,pine64-plus", "allwinner,sun50i-a64";
> > +
> > +	/* TODO: Camera, Ethernet PHY, touchscreen, etc. */
> > +};
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > new file mode 100644
> > index 000000000000..9f127b3d0e33
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > @@ -0,0 +1,74 @@
> > +/*
> > + * Copyright (c) 2016 ARM Ltd.
> > + *
> > + * This file is dual-licensed: you can use it either under the terms
> > + * of the GPL or the X11 license, at your option. Note that this dual
> > + * licensing only applies to this file, and not this project as a
> > + * whole.
> > + *
> > + *  a) This library is free software; you can redistribute it and/or
> > + *     modify it under the terms of the GNU General Public License as
> > + *     published by the Free Software Foundation; either version 2 of the
> > + *     License, or (at your option) any later version.
> > + *
> > + *     This library is distributed in the hope that it will be useful,
> > + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *     GNU General Public License for more details.
> > + *
> > + * Or, alternatively,
> > + *
> > + *  b) Permission is hereby granted, free of charge, to any person
> > + *     obtaining a copy of this software and associated documentation
> > + *     files (the "Software"), to deal in the Software without
> > + *     restriction, including without limitation the rights to use,
> > + *     copy, modify, merge, publish, distribute, sublicense, and/or
> > + *     sell copies of the Software, and to permit persons to whom the
> > + *     Software is furnished to do so, subject to the following
> > + *     conditions:
> > + *
> > + *     The above copyright notice and this permission notice shall be
> > + *     included in all copies or substantial portions of the Software.
> > + *
> > + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> > + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> > + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> > + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + *     OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sun50i-a64.dtsi"
> > +
> > +/ {
> > +	model = "Pine64";
> > +	compatible = "pine64,pine64", "allwinner,sun50i-a64";
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_pins_a>;
> > +	status = "okay";
> > +};
> > +
> > +&i2c1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&i2c1_pins>;
> > +	status = "okay";
> > +};
> > +
> > +&i2c1_pins {
> > +	allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
> 
> which should translate to:
> 	bias-pull-up;
> in the generic binding, right?

Fixed and applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161103/b004eabe/attachment.sig>

^ 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