* Re: [PATCH v2 05/11] spi: cadence-qspi: add FIFO depth detection quirk
From: Mark Brown @ 2024-04-09 15:51 UTC (permalink / raw)
To: Théo Lebrun
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vaishnav Achath,
Thomas Bogendoerfer, Rob Herring, linux-spi, devicetree,
linux-kernel, linux-mips, Vladimir Kondratiev, Gregory CLEMENT,
Thomas Petazzoni, Tawfik Bayouk
In-Reply-To: <D0FIC34Z35BV.1RT6NNGWA85SL@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 274 bytes --]
On Tue, Apr 09, 2024 at 12:07:56PM +0200, Théo Lebrun wrote:
> - (3) Make DT property optional for all compatibles.
> - (3a) If provided, warn if runtime detect value is different.
> - (3b) If provided, do not detect+warn.
I think either of these is fine.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 03/18] ASoC: dt-bindings: mt6357: Add audio codec document
From: Krzysztof Kozlowski @ 2024-04-09 15:55 UTC (permalink / raw)
To: Alexandre Mergnat, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Lee Jones, Flora Fu, Jaroslav Kysela,
Takashi Iwai, Sumit Semwal, Christian König, Catalin Marinas,
Will Deacon, Rob Herring
Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-media, dri-devel, linaro-mm-sig
In-Reply-To: <20240226-audio-i350-v3-3-16bb2c974c55@baylibre.com>
On 09/04/2024 15:42, Alexandre Mergnat wrote:
> Add MT8365 audio codec bindings to set required
> and optional voltage properties between the codec and the board.
> The properties are:
> - phandle of the requiered power supply.
typo
> - Setup of microphone bias voltage.
> - Setup of the speaker pin pull-down.
>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
> .../devicetree/bindings/sound/mt6357.yaml | 54 ++++++++++++++++++++++
Filename using compatible syntax, so missing vendor prefix.
> 1 file changed, 54 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/mt6357.yaml b/Documentation/devicetree/bindings/sound/mt6357.yaml
> new file mode 100644
> index 000000000000..381cb71b959f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mt6357.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mt6357.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek MT6357 Codec
> +
> +maintainers:
> + - Alexandre Mergnat <amergnat@baylibre.com>
> +
> +description: |
Do not need '|' unless you need to preserve formatting.
> + This is the required and optional voltage properties for this subdevice.
> + The communication between MT6357 and SoC is through Mediatek PMIC wrapper.
> + For more detail, please visit Mediatek PMIC wrapper documentation.
> + Must be a child node of PMIC wrapper.
Why?
> +
> +properties:
> +
Drop blank line
> + mediatek,hp-pull-down:
> + description:
> + Earphone driver positive output stage short to
> + the audio reference ground.
> + type: boolean
> +
> + mediatek,micbias0-microvolt:
> + description: Selects MIC Bias 0 output voltage.
> + enum: [1700000, 1800000, 1900000, 2000000,
> + 2100000, 2500000, 2600000, 2700000]
> + default: 1700000
> +
> + mediatek,micbias1-microvolt:
> + description: Selects MIC Bias 1 output voltage.
> + enum: [1700000, 1800000, 1900000, 2000000,
> + 2100000, 2500000, 2600000, 2700000]
> + default: 1700000
> +
> + mediatek,vaud28-supply:
> + description: 2.8 volt supply phandle for the audio codec
Supplies go without vendor prefixes.
> +
> +required:
> + - mediatek,vaud28-supply
That's basically no-op schema. I do not understand what you are trying
to achieve here.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + codec {
> + mediatek,micbias0-microvolt = <1900000>;
> + mediatek,micbias1-microvolt = <1700000>;
> + mediatek,vaud28-supply = <&mt6357_vaud28_reg>;
Sorry, this does not work. Change voltage to 1111111 and check the results.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 04/18] dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC
From: Krzysztof Kozlowski @ 2024-04-09 15:56 UTC (permalink / raw)
To: Alexandre Mergnat, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Lee Jones, Flora Fu, Jaroslav Kysela,
Takashi Iwai, Sumit Semwal, Christian König, Catalin Marinas,
Will Deacon, Rob Herring
Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-media, dri-devel, linaro-mm-sig
In-Reply-To: <20240226-audio-i350-v3-4-16bb2c974c55@baylibre.com>
On 09/04/2024 15:42, Alexandre Mergnat wrote:
> Add the audio codec sub-device. This sub-device is used to set required
> and optional voltage properties between the codec and the board.
>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
> Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml
> index 37423c2e0fdf..7c6a4a587b5f 100644
> --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml
> @@ -37,6 +37,11 @@ properties:
> "#interrupt-cells":
> const: 2
>
> + codec:
> + type: object
> + $ref: /schemas/sound/mt6357.yaml
> + unevaluatedProperties: false
Just put the properties here, without the codec node.
Also, your example is now incomplete.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 16/18] arm64: dts: mediatek: add mt6357 audio codec support
From: Krzysztof Kozlowski @ 2024-04-09 15:58 UTC (permalink / raw)
To: Alexandre Mergnat, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Lee Jones, Flora Fu, Jaroslav Kysela,
Takashi Iwai, Sumit Semwal, Christian König, Catalin Marinas,
Will Deacon, Rob Herring
Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-media, dri-devel, linaro-mm-sig
In-Reply-To: <20240226-audio-i350-v3-16-16bb2c974c55@baylibre.com>
On 09/04/2024 15:42, Alexandre Mergnat wrote:
> Add audio codec support of MT6357 PMIC.
> Update the file header.
Why?
>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
> arch/arm64/boot/dts/mediatek/mt6357.dtsi | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt6357.dtsi b/arch/arm64/boot/dts/mediatek/mt6357.dtsi
> index 3330a03c2f74..ade410851524 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6357.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt6357.dtsi
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: (GPL-2.0 OR MIT)
> /*
> * Copyright (c) 2020 MediaTek Inc.
> - * Copyright (c) 2023 BayLibre Inc.
> + * Copyright (c) 2024 BayLibre Inc.
That's not a reasonable change. The file was published on 2023, wasn't
it? If this is not correct, please explain why/how and make it separate
patch.
> */
>
> #include <dt-bindings/input/input.h>
> @@ -10,6 +10,9 @@ &pwrap {
> mt6357_pmic: pmic {
> compatible = "mediatek,mt6357";
>
> + mt6357_codec: codec {
> + };
There is no single point of having empty nodes.
NAK.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 18/18] arm64: dts: mediatek: add audio support for mt8365-evk
From: Krzysztof Kozlowski @ 2024-04-09 16:00 UTC (permalink / raw)
To: Alexandre Mergnat, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Lee Jones, Flora Fu, Jaroslav Kysela,
Takashi Iwai, Sumit Semwal, Christian König, Catalin Marinas,
Will Deacon, Rob Herring
Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-media, dri-devel, linaro-mm-sig
In-Reply-To: <20240226-audio-i350-v3-18-16bb2c974c55@baylibre.com>
On 09/04/2024 15:42, Alexandre Mergnat wrote:
> Add the sound node which is linked to the MT8365 SoC AFE and
> the MT6357 audio codec.
>
> Update the file header.
>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 98 +++++++++++++++++++++++++++--
> 1 file changed, 94 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
> index 50cbaefa1a99..eb0c5f076dd4 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
> @@ -1,9 +1,9 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Copyright (c) 2021-2022 BayLibre, SAS.
> - * Authors:
> - * Fabien Parent <fparent@baylibre.com>
> - * Bernhard Rosenkränzer <bero@baylibre.com>
> + * Copyright (c) 2024 BayLibre, SAS.
What is happening with your copyrights? Why do you change existing ones?
> + * Authors: Fabien Parent <fparent@baylibre.com>
> + * Bernhard Rosenkränzer <bero@baylibre.com>
> + * Alexandre Mergnat <amergnat@baylibre.com>
> */
>
> /dts-v1/;
> @@ -86,6 +86,29 @@ optee_reserved: optee@43200000 {
> reg = <0 0x43200000 0 0x00c00000>;
> };
> };
> +
> + sound: sound {
> + compatible = "mediatek,mt8365-mt6357";
> + pinctrl-names = "default",
> + "dmic",
> + "miso_off",
> + "miso_on",
> + "mosi_off",
> + "mosi_on";
> + pinctrl-0 = <&aud_default_pins>;
> + pinctrl-1 = <&aud_dmic_pins>;
> + pinctrl-2 = <&aud_miso_off_pins>;
> + pinctrl-3 = <&aud_miso_on_pins>;
> + pinctrl-4 = <&aud_mosi_off_pins>;
> + pinctrl-5 = <&aud_mosi_on_pins>;
> + mediatek,platform = <&afe>;
> + status = "okay";
Where did you disable the node?
> + };
> +};
> +
> +&afe {
> + mediatek,dmic-mode = <1>;
> + status = "okay";
> };
>
> &cpu0 {
> @@ -174,6 +197,12 @@ &mmc1 {
> status = "okay";
> };
>
> +&mt6357_codec {
> + mediatek,micbias0-microvolt = <1900000>;
> + mediatek,micbias1-microvolt = <1700000>;
> + mediatek,vaud28-supply = <&mt6357_vaud28_reg>;
> +};
> +
> &mt6357_pmic {
> interrupts-extended = <&pio 145 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-controller;
> @@ -181,6 +210,67 @@ &mt6357_pmic {
> };
>
> &pio {
> + aud_default_pins: audiodefault-pins {
> + pins {
> + pinmux = <MT8365_PIN_72_CMDAT4__FUNC_I2S3_BCK>,
> + <MT8365_PIN_73_CMDAT5__FUNC_I2S3_LRCK>,
> + <MT8365_PIN_74_CMDAT6__FUNC_I2S3_MCK>,
> + <MT8365_PIN_75_CMDAT7__FUNC_I2S3_DO>;
You have broken indentation everywhere.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]
From: Frank Li @ 2024-04-09 16:02 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
In-Reply-To: <76714850-0e02-4333-acce-02c7657666b0@linaro.org>
On Tue, Apr 09, 2024 at 08:33:22AM +0200, Krzysztof Kozlowski wrote:
> On 08/04/2024 17:36, Frank Li wrote:
> > On Fri, Apr 05, 2024 at 08:21:18PM +0200, Krzysztof Kozlowski wrote:
> >> On 05/04/2024 16:46, Frank Li wrote:
> >>> On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
> >>>> On 04/04/2024 18:19, Frank Li wrote:
> >>>>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> >>>>> i2c bus mux to connect both i2c devices. One will probe failure and other
> >>>>> will probe success when devices driver check whoami. So one dtb can cover
> >>>>> both board configuration.
> >>>>
> >>>> I don't understand it. Either you add real device or not. If one board
> >>>> has two devices, then why do you need to check for failures?
> >>>>
> >>>> Anyway, don't add fake stuff to DTS.
> >>>
> >>> NAK can't resolve the problem. It should be common problem for long time
> >>> cycle boards. Some chipes will be out life cycle. such as some sensor. So
> >>> chips on boards have been replace by some pin to pin compatible sensor. For
> >>> example:
> >>> old boards: use sensor A with address 0x1a
> >>> new bench: use sensor B with address 0x1b.
> >>>
> >>> You can treat it as two kind boards, RevA or RevB. But most user want to
> >>> use one dtb to handle such small differences. For this case, it should be
> >>> simple. Just add a super set.
> >>> i2c
> >>> {
> >>> sensorA@1a
> >>> {
> >>> }
> >>> sensorB@1b
> >>> {
> >>> }
> >>> }
> >>>
> >>> It also depend on whoami check by i2c devices. Only A or B will probe.
> >>>
> >>> wm8960 and wm8962 are more complex example. wm8960 is out of life. But
> >>> wm8962 and wm8960 have the same i2c address. The current i2c frame can't
> >>> allow the same i2c address in one i2c bus.
> >>>
> >>> You are feel to NAK my method, but I hope you also provide constructive
> >>> solution to help resolve the problem.
> >>
> >> Yes, we resolved it long time ago. Your bootloader can (usually easily)
> >> detect revision of the board and load appropriate DTS or DTS+DTSO.
> >
> > I knewn it. But the problem is one development boards A have many options,
> > so create many child dts for files, A1, A2, ... An which base on A
>
> So use DTSO, what's the problem? Other vendors, liek Rpi does not have
> problem with it and it works well. No confusion.
>
> >
> > If there are difference happen at A, create new B. then create all child
> > dtb, B1, B2, ... Bn. DTB number will increase exponent.
> >
> > If change is quite bit, we have to do that. But if change is quite small,
> > One dtb can cover it by driver auto detect, which will work like some
> > adaptor card have not plug into boards, or some sensor or NOR-flash have
> > not installed because reduce cost.
>
> You have two boards, not 20 here!
Actually, it is around ~20 derived boards. It is not upstream just because
we have not time to do that yet. After some clean up, I estimate about 7 -
- 10.
>
> >
> > Although boot loader can update dts or choose difference dts, It also cause
> > many confusition, such as layerscape, uboot update many kernel dtb's
> > information, which actually increase dependence between uboot and kernel.
> > Also it confuse people, for example, when try to debug kernel dtb, why
> > change have not token affect when change dts because not realized uboot
> > over write it.
> >
> > What's I dide is that trying to reduce unnecessary dts.
>
> There is no confusion. That's normal process, so if someone is confused
> by having variants of board, this someone will be even more confused by
> seeing non-existing hardware in his DTS.
How about existed dummy_clk and dummy regulator in dts?
>
> This problem was solved long time ago and you do not bring any
> reasonable new arguments. All vendors solved it (just look at ongoing
> discussions on board id) but only you have problem with their solution.
I never said solution was not work. Just not friend for user enough for
this case. It likes USB TypeC vs USB A port. Both works, just TypeC can't
care direction.
>
> NAK
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: adc: Add support for AD4000
From: Marcelo Schmitt @ 2024-04-09 16:09 UTC (permalink / raw)
To: David Lechner
Cc: Marcelo Schmitt, lars, Michael.Hennerich, jic23, robh+dt,
krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree,
linux-kernel
In-Reply-To: <CAMknhBEMDg3YF5pvoKJ-6y0Y5OJpmBthWfogCjy90B=F84SvzA@mail.gmail.com>
On 04/08, David Lechner wrote:
> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> >
> > Add support for AD4000 family of low noise, low power, high speed,
> > successive aproximation register (SAR) ADCs.
> >
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/iio/adc/Kconfig | 12 +
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ad4000.c | 649 +++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 663 insertions(+)
> > create mode 100644 drivers/iio/adc/ad4000.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5dfe118a5dd3..86aa96115f5a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1165,6 +1165,7 @@ L: linux-iio@vger.kernel.org
> > S: Supported
> > W: https://ez.analog.com/linux-software-drivers
> > F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > +F: drivers/iio/adc/ad4000.c
> >
> > ANALOG DEVICES INC AD4130 DRIVER
> > M: Cosmin Tanislav <cosmin.tanislav@analog.com>
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 8db68b80b391..9c9d13d4b74f 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
> > select IIO_BUFFER
> > select IIO_TRIGGERED_BUFFER
> >
> > +config AD4000
> > + tristate "Analog Devices AD4000 ADC Driver"
> > + depends on SPI
> > + select IIO_BUFFER
> > + select IIO_TRIGGERED_BUFFER
> > + help
> > + Say yes here to build support for Analog Devices AD4000 high speed
> > + SPI analog to digital converters (ADC).
> > +
> > + To compile this driver as a module, choose M here: the module will be
> > + called ad4000.
> > +
> > config AD4130
> > tristate "Analog Device AD4130 ADC Driver"
> > depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index edb32ce2af02..aa52068d864b 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -6,6 +6,7 @@
> > # When adding new entries keep the list in alphabetical order
> > obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
> > obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> > +obj-$(CONFIG_AD4000) += ad4000.o
> > obj-$(CONFIG_AD4130) += ad4130.o
> > obj-$(CONFIG_AD7091R) += ad7091r-base.o
> > obj-$(CONFIG_AD7091R5) += ad7091r5.o
> > diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> > new file mode 100644
> > index 000000000000..7997d9d98743
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad4000.c
> > @@ -0,0 +1,649 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * AD4000 SPI ADC driver
> > + *
> > + * Copyright 2024 Analog Devices Inc.
> > + */
> > +#include <asm/unaligned.h>
> > +#include <linux/bits.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/kernel.h>
> > +#include <linux/math.h>
> > +#include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/units.h>
> > +#include <linux/util_macros.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +
> > +#define AD400X_READ_COMMAND 0x54
> > +#define AD400X_WRITE_COMMAND 0x14
> > +
> > +/* AD4000 Configuration Register programmable bits */
> > +#define AD4000_STATUS BIT(4) /* Status bits output */
> > +#define AD4000_SPAN_COMP BIT(3) /* Input span compression */
> > +#define AD4000_HIGHZ BIT(2) /* High impedance mode */
> > +#define AD4000_TURBO BIT(1) /* Turbo mode */
>
> Usually bits of the same register share a similar prefix, e.g.
> AD4000_CFG_TURBO, AD4000_CFG_HIGHZ, etc.
This only has one register, but if this makes things look better will do it.
>
> > +
> > +#define AD4000_TQUIET2_NS 60
> > +
> > +#define AD4000_18BIT_MSK GENMASK(31, 14)
> > +#define AD4000_20BIT_MSK GENMASK(31, 12)
> > +
> > +#define AD4000_DIFF_CHANNEL(_sign, _real_bits) \
> > + { \
> > + .type = IIO_VOLTAGE, \
> > + .indexed = 1, \
> > + .differential = 1, \
> > + .channel = 0, \
> > + .channel2 = 1, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > + BIT(IIO_CHAN_INFO_SCALE), \
> > + .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
> > + .scan_type = { \
> > + .sign = _sign, \
> > + .realbits = _real_bits, \
> > + .storagebits = _real_bits > 16 ? 32 : 16, \
> > + .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
> > + .endianness = IIO_BE, \
> > + }, \
> > + } \
> > +
> > +#define AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits) \
> > + { \
> > + .type = IIO_VOLTAGE, \
> > + .indexed = 1, \
> > + .channel = 0, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > + BIT(IIO_CHAN_INFO_SCALE) | \
> > + BIT(IIO_CHAN_INFO_OFFSET), \
> > + .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
> > + .scan_type = { \
> > + .sign = _sign, \
> > + .realbits = _real_bits, \
> > + .storagebits = _real_bits > 16 ? 32 : 16, \
> > + .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
> > + .endianness = IIO_BE, \
> > + }, \
> > + } \
>
> It looks like all differential chips are signed and all
> pseduo-differential chips are unsigned, so I don't think we need the
> _sign parameter in these macros.
That's correct, the _sign param can be removed after the split of channel macros.
Will do it for v3.
>
> I also still have doubts about using IIO_BE and 8-bit xfers when it
> comes to adding support later to achieve max sample rate with a SPI
> offload. For example to get 2MSPS with an 18-bit chip, it will require
> an approx 33% faster SPI clock than the actual slowest clock possible
> because it will have to read 6 extra bits per sample. I didn't check
> the specs, but this may not even be physically possible without
> exceeding the datasheet max SPI clock rate. Also errors could be
> reduced if we could actually use the slowest allowable SPI clock rate.
> Furthermore, the offload hardware would have to be capable of adding
> an extra byte per sample for 18 and 20-bit chips when piping the data
> to DMA in order to get the 32-bit alignment in the buffer required by
> IIO scan_type and the natural alignment requirements of IIO buffers in
> general.
Maybe I should already implement support for reading with SPI offload
rather than doing it after this set is merged?
So we can test what happens at faster sample rates before we commit to a solution.
>
> > +
> > +enum ad4000_ids {
> > + ID_AD4000,
> > + ID_AD4001,
> > + ID_AD4002,
> > + ID_AD4003,
> > + ID_AD4004,
> > + ID_AD4005,
> > + ID_AD4006,
> > + ID_AD4007,
> > + ID_AD4008,
> > + ID_AD4010,
> > + ID_AD4011,
> > + ID_AD4020,
> > + ID_AD4021,
> > + ID_AD4022,
> > + ID_ADAQ4001,
> > + ID_ADAQ4003,
> > +};
> > +
> > +struct ad4000_chip_info {
> > + const char *dev_name;
> > + struct iio_chan_spec chan_spec;
> > +};
> > +
> > +static const struct ad4000_chip_info ad4000_chips[] = {
> > + [ID_AD4000] = {
> > + .dev_name = "ad4000",
> > + .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> > + },
> > + [ID_AD4001] = {
> > + .dev_name = "ad4001",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> > + },
> > + [ID_AD4002] = {
> > + .dev_name = "ad4002",
> > + .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> > + },
> > + [ID_AD4003] = {
> > + .dev_name = "ad4003",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > + },
> > + [ID_AD4004] = {
> > + .dev_name = "ad4004",
> > + .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> > + },
> > + [ID_AD4005] = {
> > + .dev_name = "ad4005",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> > + },
> > + [ID_AD4006] = {
> > + .dev_name = "ad4006",
> > + .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> > + },
> > + [ID_AD4007] = {
> > + .dev_name = "ad4007",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > + },
> > + [ID_AD4008] = {
> > + .dev_name = "ad4008",
> > + .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
> > + },
> > + [ID_AD4010] = {
> > + .dev_name = "ad4010",
> > + .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
> > + },
> > + [ID_AD4011] = {
> > + .dev_name = "ad4011",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > + },
> > + [ID_AD4020] = {
> > + .dev_name = "ad4020",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> > + },
> > + [ID_AD4021] = {
> > + .dev_name = "ad4021",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> > + },
> > + [ID_AD4022] = {
> > + .dev_name = "ad4022",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
> > + },
> > + [ID_ADAQ4001] = {
> > + .dev_name = "adaq4001",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
> > + },
> > + [ID_ADAQ4003] = {
> > + .dev_name = "adaq4003",
> > + .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
> > + },
> > +};
> > +
> > +enum ad4000_gains {
> > + AD4000_0454_GAIN = 0,
> > + AD4000_0909_GAIN = 1,
> > + AD4000_1_GAIN = 2,
>
> AD4000_1000_GAIN would be more consistent with the others.
Ack
>
> > + AD4000_1900_GAIN = 3,
> > + AD4000_GAIN_LEN
> > +};
> > +
> > +/*
> > + * Gains stored and computed as fractions to avoid introducing rounding errors.
> > + */
> > +static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
> > + [AD4000_0454_GAIN] = { 227, 500 },
> > + [AD4000_0909_GAIN] = { 909, 1000 },
> > + [AD4000_1_GAIN] = { 1, 1 },
> > + [AD4000_1900_GAIN] = { 19, 10 },
> > +};
>
> Why not just store the numerator in milli units and always use 1000
> for the denominator? It seems like it would simplify the code and make
> it easier to read and understand. Also, these values are coming from
> the adi,gain-milli property already, so we could avoid the enum and
> the lookup table entirely and simplify things even more.
Makes sense. Will do it.
>
> > +
> > +struct ad4000_state {
> > + struct spi_device *spi;
> > + struct gpio_desc *cnv_gpio;
> > + int vref;
> > + bool status_bits;
> > + bool span_comp;
> > + bool turbo_mode;
> > + bool high_z_mode;
> > +
> > + enum ad4000_gains pin_gain;
> > + int scale_tbl[AD4000_GAIN_LEN][2][2];
> > +
> > + /*
> > + * DMA (thus cache coherency maintenance) requires the
> > + * transfer buffers to live in their own cache lines.
> > + */
> > + struct {
> > + union {
> > + u16 sample_buf16;
> > + u32 sample_buf32;
>
> Technically, these are holding big-endian data, so __be16 and __be32
> would be more correct.
Ack
>
> > + } data;
> > + s64 timestamp __aligned(8);
> > + } scan;
> > + __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
> > + __be16 rx_buf;
> > +};
>
> scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs
> to be moved to the scan field.
I have already tried it. Maybe I did something wrong besides buffer alignment
at that time. Will give it another try.
>
> > +
> > +static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits,
> > + const struct ad4000_chip_info *chip)
> > +{
> > + int diff = chip->chan_spec.differential;
> > + int val, val2, tmp0, tmp1, i;
> > + u64 tmp2;
> > +
> > + val2 = scale_bits;
> > + for (i = 0; i < AD4000_GAIN_LEN; i++) {
>
> Only one gain is selected by the devicetree, so why do we need to do
> this for all 4 gains?
>
Good point. Will think better how to simplify this.
> > + val = st->vref / 1000;
> > + /* Multiply by MILLI here to avoid losing precision */
> > + val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
> > + ad4000_gains_frac[i][0]);
> > + /* Would multiply by NANO here but we already multiplied by MILLI */
> > + tmp2 = shift_right((u64)val * MICRO, val2);
> > + tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1);
> > + /* Store scale for when span compression is disabled */
> > + st->scale_tbl[i][0][0] = tmp0; /* Integer part */
> > + st->scale_tbl[i][0][1] = abs(tmp1); /* Fractional part */
> > + /* Store scale for when span compression is enabled */
> > + st->scale_tbl[i][1][0] = tmp0;
> > + if (diff)
> > + st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 4, 5);
> > + else
> > + st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 9, 10);
> > + }
> > +}
> > +
> > +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
> > +{
> > + put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val,
> > + &st->tx_buf);
> > + return spi_write(st->spi, &st->tx_buf, 2);
> > +}
> > +
> > +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
> > +{
> > + struct spi_transfer t[] = {
> > + {
> > + .tx_buf = &st->tx_buf,
> > + .rx_buf = &st->rx_buf,
> > + .len = 2,
> > + },
> > + };
> > + int ret;
> > +
> > + put_unaligned_be16(AD400X_READ_COMMAND << BITS_PER_BYTE, &st->tx_buf);
> > + ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = get_unaligned_be16(&st->rx_buf);
> > +
> > + return ret;
> > +}
> > +
>
> It would be very helpful to have comments here explaining the exact
> expected wiring configuration and signal timing here since there are
> so many possibilities for this chip.
>
Ok, will be spliting this into different handling for the wiring modes so
will add comments to make clear what supports each configuration.
> > +static int ad4000_read_sample(struct ad4000_state *st,
> > + const struct iio_chan_spec *chan)
> > +{
> > + struct spi_transfer t[] = {
>
> Don't really need [] here since there is only one xfer.
>
Ack
> > + {
> > + .rx_buf = &st->scan.data,
> > + .len = BITS_TO_BYTES(chan->scan_type.storagebits),
> > + .delay = {
> > + .value = AD4000_TQUIET2_NS,
> > + .unit = SPI_DELAY_UNIT_NSECS,
> > + },
> > + },
> > + };
> > + int ret;
> > +
> > + ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int ad4000_single_conversion(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan, int *val)
> > +{
> > + struct ad4000_state *st = iio_priv(indio_dev);
> > + u32 sample;
> > + int ret;
> > +
> > + if (st->cnv_gpio)
> > + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);
>
> It would make more sense and be less redundant to move the gpio code
> into ad4000_read_sample().
>
> Also, gpiod_set_value_cansleep() checks for NULL, so the if () is redundant.
>
Good point. I think the execution flow migth change a bit here but will try
to avoid things like that.
> > +
> > + ret = ad4000_read_sample(st, chan);
> > + if (ret)
> > + return ret;
> > +
> > + if (st->cnv_gpio)
> > + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
> > +
> > + if (chan->scan_type.storagebits > 16)
> > + sample = get_unaligned_be32(&st->scan.data);
> > + else
> > + sample = get_unaligned_be16(&st->scan.data);
>
> data is aligned, so be32/16_to_cpu() should be fine. Also, Jonathan
> will suggest to use &st->scan.data.sample_b32/16 here too. :-)
Ack
>
> > +
> > + switch (chan->scan_type.realbits) {
> > + case 16:
> > + break;
> > + case 18:
> > + sample = FIELD_GET(AD4000_18BIT_MSK, sample);
> > + break;
> > + case 20:
> > + sample = FIELD_GET(AD4000_20BIT_MSK, sample);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (chan->scan_type.sign == 's')
> > + *val = sign_extend32(sample, chan->scan_type.realbits - 1);
> > +
> > + return IIO_VAL_INT;
> > +}
> > +
> > +static int ad4000_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan, int *val,
> > + int *val2, long info)
> > +{
> > + struct ad4000_state *st = iio_priv(indio_dev);
> > +
> > + switch (info) {
> > + case IIO_CHAN_INFO_RAW:
> > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> > + return ad4000_single_conversion(indio_dev, chan, val);
> > + unreachable();
> > + case IIO_CHAN_INFO_SCALE:
> > + *val = st->scale_tbl[st->pin_gain][st->span_comp][0];
> > + *val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
> > + return IIO_VAL_INT_PLUS_NANO;
> > + case IIO_CHAN_INFO_OFFSET:
> > + *val = 0;
> > + if (st->span_comp)
> > + *val = mult_frac(st->vref / 1000, 1, 10);
> > +
> > + return IIO_VAL_INT;
> > + default:
> > + break;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int ad4000_read_avail(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + const int **vals, int *type, int *length,
> > + long info)
> > +{
> > + struct ad4000_state *st = iio_priv(indio_dev);
> > +
> > + switch (info) {
> > + case IIO_CHAN_INFO_SCALE:
> > + *vals = (int *)st->scale_tbl[st->pin_gain];
> > + *length = 2 * 2;
> > + *type = IIO_VAL_INT_PLUS_NANO;
> > + return IIO_AVAIL_LIST;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static int ad4000_write_raw_get_fmt(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan, long mask)
> > +{
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SCALE:
> > + return IIO_VAL_INT_PLUS_NANO;
> > + default:
> > + return IIO_VAL_INT_PLUS_MICRO;
> > + }
> > + return -EINVAL;
>
> not reachable because of default, so can be left out
>
Ack
> > +}
> > +
> > +static int ad4000_write_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan, int val, int val2,
> > + long mask)
> > +{
> > + struct ad4000_state *st = iio_priv(indio_dev);
> > + unsigned int reg_val;
> > + bool span_comp_en;
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SCALE:
> > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> > + ret = ad4000_read_reg(st, ®_val);
> > + if (ret < 0)
> > + return ret;
> > +
> > + span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
> > + reg_val &= ~AD4000_SPAN_COMP;
> > + reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
> > +
> > + ret = ad4000_write_reg(st, reg_val);
> > + if (ret < 0)
> > + return ret;
> > +
> > + st->span_comp = span_comp_en;
> > + return 0;
> > + }
> > + unreachable();
>
> Can bring out the return 0 to avoid unreachable.
Ack
>
> > + default:
> > + break;
>
> Can return -EINVAL to avoid break;
>
Ack
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static irqreturn_t ad4000_trigger_handler(int irq, void *p)
> > +{
> > + struct iio_poll_func *pf = p;
> > + struct iio_dev *indio_dev = pf->indio_dev;
> > + struct ad4000_state *st = iio_priv(indio_dev);
> > + int ret;
> > +
> > + if (st->cnv_gpio)
> > + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
> > +
> > + ret = ad4000_read_sample(st, &indio_dev->channels[0]);
> > + if (ret < 0)
> > + goto err_out;
> > +
> > + if (st->cnv_gpio)
> > + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
> > +
> > + iio_push_to_buffers_with_timestamp(indio_dev, &st->scan,
> > + iio_get_time_ns(indio_dev));
> > +
> > +err_out:
> > + iio_trigger_notify_done(indio_dev->trig);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static const struct iio_info ad4000_info = {
> > + .read_raw = &ad4000_read_raw,
> > + .read_avail = &ad4000_read_avail,
> > + .write_raw = &ad4000_write_raw,
> > + .write_raw_get_fmt = &ad4000_write_raw_get_fmt,
> > +};
> > +
> > +static void ad4000_config(struct ad4000_state *st)
> > +{
> > + unsigned int reg_val;
> > + int ret;
> > +
> > + reg_val = FIELD_PREP(AD4000_TURBO, 1);
>
> Since the driver in it's current state can get anywhere near the max
> sample rate of ~1MSPS, I don't think it makes sense to enable turbo at
> this point.
>
This is just enabling turbo at start up. If not enabling turbo during probe,
we would want(need?) to provide some interface for that, which might not be
much desired.
> > +
> > + if (device_property_present(&st->spi->dev, "adi,high-z-input"))
> > + reg_val |= FIELD_PREP(AD4000_HIGHZ, 1);
> > +
> > + /*
> > + * The ADC SDI pin might be connected to controller CS line in which
> > + * case the write might fail. This, however, does not prevent the device
> > + * from functioning even though in a configuration other than the
> > + * requested one.
> > + */
> > + ret = ad4000_write_reg(st, reg_val);
> > + if (ret < 0)
> > + dev_dbg(&st->spi->dev, "Failed to config device\n");
>
> If writing fails because there is no CS line wired up, we won't get an
> error returned here. The SPI controller has no way of knowing this
> happened, so it can only assume the write was successful and return 0.
> So this should return ret.
>
Ok, ack.
> Ideally, the devicetree should tell us if CS is wired up or not.
>
> > +}
> > +
> > +static void ad4000_regulator_disable(void *reg)
> > +{
> > + regulator_disable(reg);
> > +}
> > +
> > +static int ad4000_probe(struct spi_device *spi)
> > +{
> > + const struct ad4000_chip_info *chip;
> > + struct regulator *vref_reg;
> > + struct iio_dev *indio_dev;
> > + struct ad4000_state *st;
> > + int ret;
>
> We need a check somewhere in here to make sure that adi,spi-mode is in
> a supported configuration. E.g. chain mode is not currently
> implemented.
ok, will add that.
>
> > +
> > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > + if (!indio_dev)
> > + return -ENOMEM;
> > +
> > + chip = spi_get_device_match_data(spi);
> > + if (!chip)
> > + return -EINVAL;
> > +
> > + st = iio_priv(indio_dev);
> > + st->spi = spi;
> > +
> > + ret = devm_regulator_get_enable(&spi->dev, "vdd");
> > + if (ret)
> > + return dev_err_probe(&spi->dev, ret, "Failed to enable VDD supply\n");
> > +
> > + ret = devm_regulator_get_enable(&spi->dev, "vio");
> > + if (ret)
> > + return dev_err_probe(&spi->dev, ret, "Failed to enable VIO supply\n");
> > +
> > + vref_reg = devm_regulator_get(&spi->dev, "ref");
> > + if (IS_ERR(vref_reg))
> > + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
> > + "Failed to get vref regulator\n");
> > +
> > + ret = regulator_enable(vref_reg);
> > + if (ret < 0)
> > + return dev_err_probe(&spi->dev, ret,
> > + "Failed to enable voltage regulator\n");
> > +
> > + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
> > + if (ret)
> > + return dev_err_probe(&spi->dev, ret,
> > + "Failed to add regulator disable action\n");
> > +
> > + st->vref = regulator_get_voltage(vref_reg);
> > + if (st->vref < 0)
> > + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
> > +
> > + st->cnv_gpio = devm_gpiod_get_optional(&spi->dev, "cnv", GPIOD_OUT_HIGH);
> > + if (IS_ERR(st->cnv_gpio)) {
> > + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
>
> EPROBE_DEFER check is not needed with dev_err_probe();, it already does that.
Ack
>
>
> > +
> > + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
> > + "Failed to get CNV GPIO");
> > + }
> > +
> > + ad4000_config(st);
> > +
> > + indio_dev->name = chip->dev_name;
> > + indio_dev->info = &ad4000_info;
> > + indio_dev->channels = &chip->chan_spec;
> > + indio_dev->num_channels = 1;
> > +
> > + st->pin_gain = AD4000_1_GAIN;
> > + if (device_property_present(&spi->dev, "adi,gain-milli")) {
> > + u32 val;
>
> Should it be an error if adi,gain-milli is set on non-adaq chips?
Maybe. We should not change the scale if it's a chip that don't have the
amplifier in front of the ADC. I think the best handling would be to just
ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT
constraint,
- if:
properties:
compatible:
contains:
enum:
- adi,adaq4001
- adi,adaq4003
then:
properties:
adi,gain-milli: false
?
>
> > +
> > + ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
> > + if (ret)
> > + return ret;
> > +
> > + switch (val) {
> > + case 454:
> > + st->pin_gain = AD4000_0454_GAIN;
> > + break;
> > + case 909:
> > + st->pin_gain = AD4000_0909_GAIN;
> > + break;
> > + case 1000:
> > + st->pin_gain = AD4000_1_GAIN;
> > + break;
> > + case 1900:
> > + st->pin_gain = AD4000_1900_GAIN;
> > + break;
> > + default:
> > + return dev_err_probe(&spi->dev, -EINVAL,
> > + "Invalid firmware provided gain\n");
>
> Could help debugging if val is included in the error message.
Ack
>
>
> > + }
> > + }
> > +
> > + /*
> > + * ADCs that output twos complement code have one less bit to express
> > + * voltage magnitude.
> > + */
> > + if (chip->chan_spec.scan_type.sign == 's')
> > + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1,
> > + chip);
> > + else
> > + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits,
> > + chip);
> > +
> > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> > + &iio_pollfunc_store_time,
> > + &ad4000_trigger_handler, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + return devm_iio_device_register(&spi->dev, indio_dev);
> > +}
> > +
> > +static const struct spi_device_id ad4000_id[] = {
> > + { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
> > + { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
> > + { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
> > + { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
> > + { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
> > + { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
> > + { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
> > + { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
> > + { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
> > + { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
> > + { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
> > + { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
> > + { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
> > + { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
> > + { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
> > + { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(spi, ad4000_id);
> > +
> > +static const struct of_device_id ad4000_of_match[] = {
> > + { .compatible = "adi,ad4000", .data = &ad4000_chips[ID_AD4000] },
> > + { .compatible = "adi,ad4001", .data = &ad4000_chips[ID_AD4001] },
> > + { .compatible = "adi,ad4002", .data = &ad4000_chips[ID_AD4002] },
> > + { .compatible = "adi,ad4003", .data = &ad4000_chips[ID_AD4003] },
> > + { .compatible = "adi,ad4004", .data = &ad4000_chips[ID_AD4004] },
> > + { .compatible = "adi,ad4005", .data = &ad4000_chips[ID_AD4005] },
> > + { .compatible = "adi,ad4006", .data = &ad4000_chips[ID_AD4006] },
> > + { .compatible = "adi,ad4007", .data = &ad4000_chips[ID_AD4007] },
> > + { .compatible = "adi,ad4008", .data = &ad4000_chips[ID_AD4008] },
> > + { .compatible = "adi,ad4010", .data = &ad4000_chips[ID_AD4010] },
> > + { .compatible = "adi,ad4011", .data = &ad4000_chips[ID_AD4011] },
> > + { .compatible = "adi,ad4020", .data = &ad4000_chips[ID_AD4020] },
> > + { .compatible = "adi,ad4021", .data = &ad4000_chips[ID_AD4021] },
> > + { .compatible = "adi,ad4022", .data = &ad4000_chips[ID_AD4022] },
> > + { .compatible = "adi,adaq4001", .data = &ad4000_chips[ID_ADAQ4001] },
> > + { .compatible = "adi,adaq4003", .data = &ad4000_chips[ID_ADAQ4003] },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ad4000_of_match);
> > +
> > +static struct spi_driver ad4000_driver = {
> > + .driver = {
> > + .name = "ad4000",
> > + .of_match_table = ad4000_of_match,
> > + },
> > + .probe = ad4000_probe,
> > + .id_table = ad4000_id,
> > +};
> > +module_spi_driver(ad4000_driver);
> > +
> > +MODULE_AUTHOR("Mircea Caprioru <mircea.caprioru@analog.com>");
> > +MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>");
> > +MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >
> >
^ permalink raw reply
* Re: [PATCH v7 2/3] dt-bindings: pinctrl: Document nuvoton ma35d1 pin control
From: Rob Herring @ 2024-04-09 16:29 UTC (permalink / raw)
To: Jacky Huang
Cc: linus.walleij, krzysztof.kozlowski+dt, conor+dt, p.zabel,
j.neuschaefer, linux-arm-kernel, linux-gpio, devicetree,
linux-kernel, ychuang3, schung, Krzysztof Kozlowski
In-Reply-To: <20240409095637.2135-3-ychuang570808@gmail.com>
On Tue, Apr 09, 2024 at 09:56:36AM +0000, Jacky Huang wrote:
> From: Jacky Huang <ychuang3@nuvoton.com>
>
> Add documentation to describe nuvoton ma35d1 pin control and GPIO.
>
> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> .../pinctrl/nuvoton,ma35d1-pinctrl.yaml | 163 ++++++++++++++++++
> 1 file changed, 163 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml
> new file mode 100644
> index 000000000000..8b9ec263213f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml
> @@ -0,0 +1,163 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,ma35d1-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton MA35D1 pin control and GPIO
> +
> +maintainers:
> + - Shan-Chun Hung <schung@nuvoton.com>
> + - Jacky Huang <ychuang3@nuvoton.com>
> +
> +allOf:
> + - $ref: pinctrl.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - nuvoton,ma35d1-pinctrl
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 1
> +
> + nuvoton,sys:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + phandle of the system-management node.
If these are the *only* registers to access the pinctrl functions, then
this binding should be a child node of the system-management node and
then you don't need this property.
And if the registers for pinctrl are a defined range, you should add a
'reg' property (even though Linux and regmap don't use it).
> +
> + ranges: true
This property makes no sense with the binding as-is. You don't have
any address to translate. Maybe with the above changes it will.
> +
> +patternProperties:
> + "^gpio@[0-9a-f]+$":
> + type: object
> + additionalProperties: false
> + properties:
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + interrupt-controller: true
> +
> + '#interrupt-cells':
> + const: 2
> +
> + interrupts:
> + description:
> + The interrupt outputs to sysirq.
> + maxItems: 1
> +
> + required:
> + - gpio-controller
> + - '#gpio-cells'
> + - reg
> + - clocks
> + - interrupt-controller
> + - '#interrupt-cells'
> + - interrupts
> +
> + "^pin-[a-z0-9]+$":
> + type: object
> + description:
> + A pinctrl node should contain at least one subnodes representing the
> + pinctrl groups available on the machine. Each subnode will list the
> + pins it needs, and how they should be configured, with regard to muxer
> + configuration, pullups, drive strength, input enable/disable and input
> + schmitt.
> +
> + $ref: pincfg-node.yaml#
> +
> + properties:
> + power-source:
> + description: |
> + Valid arguments are described as below:
> + 0: power supply of 1.8V
> + 1: power supply of 3.3V
> + enum: [0, 1]
> +
> + drive-strength-microamp:
> + oneOf:
> + - enum: [ 2900, 4400, 5800, 7300, 8600, 10100, 11500, 13000 ]
> + description: 1.8V I/O driving strength
> + - enum: [ 17100, 25600, 34100, 42800, 48000, 56000, 77000, 82000 ]
> + description: 3.3V I/O driving strength
> +
> + unevaluatedProperties: false
In the indented cases, it's preferred to put this before 'properties'.
> +
> + "-grp$":
> + type: object
> + description:
> + Pinctrl node's client devices use subnodes for desired pin configuration.
> + Client device subnodes use below standard properties.
Missing $ref to common properties and 'unevaluatedProperties'.
> + properties:
> + nuvoton,pins:
> + description:
> + Each entry consists of 4 parameters and represents the mux and config
> + setting for one pin.
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + minItems: 1
> + items:
> + items:
> + - minimum: 0
> + maximum: 13
> + description:
> + Pin bank.
> + - minimum: 0
> + maximum: 15
> + description:
> + Pin bank index.
> + - minimum: 0
> + maximum: 15
> + description:
> + Mux 0 means GPIO and mux 1 to 15 means the specific device function.
> +
> +required:
> + - compatible
> + - nuvoton,sys
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> +
> + pinctrl@40040000 {
> + compatible = "nuvoton,ma35d1-pinctrl";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + nuvoton,sys = <&sys>;
> + ranges = <0 0x40040000 0xc00>;
> +
> + gpio@0 {
> + reg = <0x0 0x40>;
> + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk GPA_GATE>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> +
> + uart-grp {
> + uart11-pins {
This is not what the schema says.
> + nuvoton,pins = <11 0 2>,
> + <11 1 2>,
> + <11 2 2>,
> + <11 3 2>;
> + bias-disable;
> + power-source = <1>;
> + };
> + };
Include a pin-* node in the example.
> + };
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v6 4/4] drivers: watchdog: ast2500 and ast2600 support bootstatus
From: PeterYin @ 2024-04-09 16:28 UTC (permalink / raw)
To: patrick, Wim Van Sebroeck, Guenter Roeck, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
In-Reply-To: <20240328022231.3649741-5-peteryin.openbmc@gmail.com>
Peter Yin 於 3/28/24 10:22 寫道:
> Add WDIOF_EXTERN1 and WDIOF_CARDRESET bootstatus in ast2600
>
> Regarding the AST2600 specification, the WDTn Timeout Status Register
> (WDT10) has bit 1 reserved. Bit 1 of the status register indicates
> on ast2500 if the boot was from the second boot source.
> It does not indicate that the most recent reset was triggered by
> the watchdog. The code should just be changed to set WDIOF_CARDRESET
> if bit 0 of the status register is set.
>
> Include SCU register to veriy WDIOF_EXTERN1 in ast2600 SCU74 or
> ast2500 SCU3C when bit1 is set.
>
> Signed-off-by: Peter Yin <peteryin.openbmc@gmail.com>
> ---
> drivers/watchdog/aspeed_wdt.c | 35 +++++++++++++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index b4773a6aaf8c..0e7ef860cbdc 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -11,10 +11,12 @@
> #include <linux/io.h>
> #include <linux/kernel.h>
> #include <linux/kstrtox.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <linux/platform_device.h>
> +#include <linux/regmap.h>
> #include <linux/watchdog.h>
>
> static bool nowayout = WATCHDOG_NOWAYOUT;
> @@ -77,11 +79,19 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
> #define WDT_TIMEOUT_STATUS 0x10
> #define WDT_TIMEOUT_STATUS_IRQ BIT(2)
> #define WDT_TIMEOUT_STATUS_BOOT_SECONDARY BIT(1)
> +#define WDT_TIMEOUT_STATUS_EVENT BIT(0)
> #define WDT_CLEAR_TIMEOUT_STATUS 0x14
> #define WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION BIT(0)
> #define WDT_RESET_MASK1 0x1c
> #define WDT_RESET_MASK2 0x20
>
> +/*
> + * Ast2600 SCU74 bit1 is External reset flag
> + * Ast2500 SCU3C bit1 is External reset flag
> + */
> +#define AST2500_SYSTEM_RESET_EVENT 0x3C
> +#define AST2600_SYSTEM_RESET_EVENT 0x74
> +#define EXTERN_RESET_FLAG BIT(1)
> /*
> * WDT_RESET_WIDTH controls the characteristics of the external pulse (if
> * enabled), specifically:
> @@ -330,6 +340,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> if (IS_ERR(wdt->base))
> return PTR_ERR(wdt->base);
>
> + struct regmap *scu_base = syscon_regmap_lookup_by_phandle(dev->of_node,
> + "aspeed,scu");
> + if (IS_ERR(scu_base))
> + return PTR_ERR(scu_base);
> +
> wdt->wdd.info = &aspeed_wdt_info;
>
> if (wdt->cfg->irq_mask) {
> @@ -459,14 +474,26 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> }
>
> status = readl(wdt->base + WDT_TIMEOUT_STATUS);
> - if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
> + if (status & WDT_TIMEOUT_STATUS_EVENT)
> wdt->wdd.bootstatus = WDIOF_CARDRESET;
>
> - if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
> - of_device_is_compatible(np, "aspeed,ast2500-wdt"))
> - wdt->wdd.groups = bswitch_groups;
> + if (of_device_is_compatible(np, "aspeed,ast2600-wdt")) {
> + ret = regmap_read(scu_base,
> + AST2600_SYSTEM_RESET_EVENT,
> + &status);
> + } else {
> + ret = regmap_read(scu_base,
> + AST2500_SYSTEM_RESET_EVENT,
> + &status);
> + wdt->wdd.groups = bswitch_groups;
> }
>
> + /*
> + * Reset cause by Extern Reset
> + */
> + if (status & EXTERN_RESET_FLAG && !ret)
> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> +
> dev_set_drvdata(dev, wdt);
>
> return devm_watchdog_register_device(dev, &wdt->wdd);
Hi Guenter,
Could you help me understand the definition of WDIOF_CARDRESET in
the kernel? If it resets the CPU, should all values be reset to default?
Should we check the POR (RstPwr Power on reset SRST# flag) flag in SCU
0x74 register bit 0 in ast2600?
^ permalink raw reply
* Re: [PATCH v2 0/2] Add support for AD4000 series
From: David Lechner @ 2024-04-09 16:31 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: Marcelo Schmitt, lars, Michael.Hennerich, jic23, robh+dt,
krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree,
linux-kernel
In-Reply-To: <ZhVX76dVt-TrC0NX@debian-BULLSEYE-live-builder-AMD64>
On Tue, Apr 9, 2024 at 9:59 AM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> wrote:
>
> On 04/08, David Lechner wrote:
> > On Mon, Apr 8, 2024 at 9:31 AM Marcelo Schmitt
> > <marcelo.schmitt@analog.com> wrote:
> > >
...
> > >
> > > - Why did not make vref regulator optional?
> > > Other SAR ADCs I've seen needed a voltage reference otherwise they simply
> > > could not provide any reasonable readings. Isn't it preferable to fail rather
> > > than having a device that can't provide reliable data?
> >
> > In the device tree bindings, making vref-supply required makes sense
> > since there is no internal reference. In the driver, as discussed in
> > V1, it will fail if vref-supply in regulator_get_voltage() if
> > vref-supply is missing and we use devm_regulator_get() instead of
> > devm_regulator_get_optional(). So leaving it as-is is fine. We have a
> > plan to clean this up later anyway.
> >
>
> Not sure I understand the idea here. Should the driver use
> devm_regulator_get_optional() instead of devm_regulator_get() because
> the optional call would fail immediately if no vref-supply while the regular
> call would only fail at regulator_get_voltage()? Why? This looks very counter
> intuitive to me.
Right. I'm saying just leave it the way it is for now.
(I have a plan to simplify it later, but still working on that.)
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: adc: Add support for AD4000
From: David Lechner @ 2024-04-09 16:44 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: Marcelo Schmitt, lars, Michael.Hennerich, jic23, robh+dt,
krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree,
linux-kernel
In-Reply-To: <ZhVoTi2amNTOJ4eS@debian-BULLSEYE-live-builder-AMD64>
On Tue, Apr 9, 2024 at 11:09 AM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> wrote:
>
> On 04/08, David Lechner wrote:
> > On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > <marcelo.schmitt@analog.com> wrote:
> > >
...
> >
> > I also still have doubts about using IIO_BE and 8-bit xfers when it
> > comes to adding support later to achieve max sample rate with a SPI
> > offload. For example to get 2MSPS with an 18-bit chip, it will require
> > an approx 33% faster SPI clock than the actual slowest clock possible
> > because it will have to read 6 extra bits per sample. I didn't check
> > the specs, but this may not even be physically possible without
> > exceeding the datasheet max SPI clock rate. Also errors could be
> > reduced if we could actually use the slowest allowable SPI clock rate.
> > Furthermore, the offload hardware would have to be capable of adding
> > an extra byte per sample for 18 and 20-bit chips when piping the data
> > to DMA in order to get the 32-bit alignment in the buffer required by
> > IIO scan_type and the natural alignment requirements of IIO buffers in
> > general.
>
> Maybe I should already implement support for reading with SPI offload
> rather than doing it after this set is merged?
> So we can test what happens at faster sample rates before we commit to a solution.
>
Yes, that sounds like a wise thing to do.
>
> >
> > > + } data;
> > > + s64 timestamp __aligned(8);
> > > + } scan;
> > > + __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
> > > + __be16 rx_buf;
> > > +};
> >
> > scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs
> > to be moved to the scan field.
>
> I have already tried it. Maybe I did something wrong besides buffer alignment
> at that time. Will give it another try.
This is the alignment for DMA cache coherency. So it should not have
any affect on the actual data read, only performance.
> > > +static void ad4000_config(struct ad4000_state *st)
> > > +{
> > > + unsigned int reg_val;
> > > + int ret;
> > > +
> > > + reg_val = FIELD_PREP(AD4000_TURBO, 1);
> >
> > Since the driver in it's current state can get anywhere near the max
> > sample rate of ~1MSPS, I don't think it makes sense to enable turbo at
> > this point.
> >
>
> This is just enabling turbo at start up. If not enabling turbo during probe,
> we would want(need?) to provide some interface for that, which might not be
> much desired.
>
TURBO is only needed to achieve the max sample rate of 500k/1M/2MSPS
on the various chips by skipping powering down some circuitry between
samples. We can't get anywhere close to that in Linux without some
sort of SPI offloading. So, for now, we might as well leave it
disabled and save some power.
> > > +
> > > + st->pin_gain = AD4000_1_GAIN;
> > > + if (device_property_present(&spi->dev, "adi,gain-milli")) {
> > > + u32 val;
> >
> > Should it be an error if adi,gain-milli is set on non-adaq chips?
>
> Maybe. We should not change the scale if it's a chip that don't have the
> amplifier in front of the ADC. I think the best handling would be to just
> ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT
> constraint,
> - if:
> properties:
> compatible:
> contains:
> enum:
> - adi,adaq4001
> - adi,adaq4003
> then:
> properties:
> adi,gain-milli: false
> ?
I think this is missing a not:, but otherwise yes this should be in
the DT bindings.
Even with that though, I would still be helpful to readers of the
driver to at least have a comment here pointing out that this property
and related gain scaling only applies to ADAQ chips.
^ permalink raw reply
* Re: [PATCH v1] arm64: dts: imx8mm: fix missing pgc_vpu_* power domain parent
From: Vitor Soares @ 2024-04-09 16:44 UTC (permalink / raw)
To: Lucas Stach, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Vitor Soares, devicetree, imx, linux-arm-kernel, linux-kernel,
stable
In-Reply-To: <fcd6acc268b8642371cf289149b2b1c3e90c7f45.camel@pengutronix.de>
On Tue, 2024-04-09 at 16:36 +0200, Lucas Stach wrote:
> Am Dienstag, dem 09.04.2024 um 14:22 +0100 schrieb Vitor Soares:
> > Hi Lucas,
> >
> > Thanks for your feedback.
> >
> > On Tue, 2024-04-09 at 11:13 +0200, Lucas Stach wrote:
> > > Hi Vitor,
> > >
> > > Am Dienstag, dem 09.04.2024 um 09:58 +0100 schrieb Vitor Soares:
> > > > From: Vitor Soares <vitor.soares@toradex.com>
> > > >
> > > > The pgc_vpu_* nodes miss the reference to the power domain
> > > > parent,
> > > > leading the system to hang during the resume.
> > > >
> > > This change is not correct. The vpumix domain is controlled
> > > through
> > > the
> > > imx8mm-vpu-blk-ctrl and must not be directly triggered by the
> > > child
> > > domains in order to guarantee proper power sequencing.
> > >
> > > If the sequencing is incorrect for resume, it needs to be fixed
> > > in
> > > the
> > > blk-ctrl driver. I'll happily assist if you have any questions
> > > about
> > > this intricate mix between GPC and blk-ctrl hardware/drivers.
> >
> > I'm new into the topic, so I tried to follow same approach as in
> > imx8mp
> > DT.
> >
> That's a good hint, the 8MP VPU GPC node additions missed my radar.
> The
> direct dependency there between the GPC domains is equally wrong.
>
> > I also checked the imx8mq DT and it only have one domain for the
> > VPU in the GPC. It seem blk-ctrl also dependes on pgc_vpu_* to work
> > properly.
> >
> > The blk-ctrl driver hangs on imx8m_blk_ctrl_power_on() when access
> > the
> > ip registers for the soft reset. I tried to power-up the before the
> > soft reset, but it didn't work.
> >
> The runtime_pm_get_sync() at the start of that function should ensure
> that bus GPC domain aka vpumix is powered up. Can you check if that
> is
> happening?
I checked bc->bus_power_dev->power.runtime_status and it is RPM_ACTIVE.
Am I looking to on the right thing? It is RPM_ACTIVE event before
runtime_pm_get_sync().
>
> Regards,
> Lucas
>
> > Do you have an idea how we can address this within blk-ctrl?
> >
> > Best regards,
> > Vitor
^ permalink raw reply
* [PATCH] arm64: dts: ti: iot2050: Add icssg-prueth nodes for PG1 devices
From: Diogo Ivo @ 2024-04-09 16:43 UTC (permalink / raw)
To: nm, vigneshr, kristo, robh, krzysztof.kozlowski+dt, conor+dt,
linux-arm-kernel, devicetree
Cc: Jan Kiszka, diogo.ivo
From: Jan Kiszka <jan.kiszka@siemens.com>
Add the required nodes to enable ICSSG SR1.0 based prueth networking.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
---
.../dts/ti/k3-am65-iot2050-common-pg1.dtsi | 32 ++++++++++++++++---
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
index c50a585dd638..ef7897763ef8 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
@@ -43,9 +43,33 @@ &tx_pru2_1 {
};
&icssg0_eth {
- status = "disabled";
-};
+ compatible = "ti,am654-sr1-icssg-prueth";
-&icssg0_mdio {
- status = "disabled";
+ ti,prus = <&pru0_0>, <&rtu0_0>, <&pru0_1>, <&rtu0_1>;
+ firmware-name = "ti-pruss/am65x-pru0-prueth-fw.elf",
+ "ti-pruss/am65x-rtu0-prueth-fw.elf",
+ "ti-pruss/am65x-pru1-prueth-fw.elf",
+ "ti-pruss/am65x-rtu1-prueth-fw.elf";
+
+ ti,pruss-gp-mux-sel = <2>, /* MII mode */
+ <2>,
+ <2>, /* MII mode */
+ <2>;
+
+ dmas = <&main_udmap 0xc100>, /* egress slice 0 */
+ <&main_udmap 0xc101>, /* egress slice 0 */
+ <&main_udmap 0xc102>, /* egress slice 0 */
+ <&main_udmap 0xc103>, /* egress slice 0 */
+ <&main_udmap 0xc104>, /* egress slice 1 */
+ <&main_udmap 0xc105>, /* egress slice 1 */
+ <&main_udmap 0xc106>, /* egress slice 1 */
+ <&main_udmap 0xc107>, /* egress slice 1 */
+ <&main_udmap 0x4100>, /* ingress slice 0 */
+ <&main_udmap 0x4101>, /* ingress slice 1 */
+ <&main_udmap 0x4102>, /* mgmnt rsp slice 0 */
+ <&main_udmap 0x4103>; /* mgmnt rsp slice 1 */
+ dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
+ "tx1-0", "tx1-1", "tx1-2", "tx1-3",
+ "rx0", "rx1",
+ "rxmgm0", "rxmgm1";
};
--
2.44.0
^ permalink raw reply related
* [PATCH v4 0/5] virt: vmgenid: Add devicetree bindings support
From: Sudan Landge @ 2024-04-09 17:01 UTC (permalink / raw)
To: tytso, Jason, robh+dt, krzysztof.kozlowski+dt, conor+dt, sudanl,
sathyanarayanan.kuppuswamy, thomas.lendacky, dan.j.williams,
devicetree, linux-kernel
Cc: graf, dwmw, bchalios, xmarcalx
This small series of patches aims to add devicetree bindings support for
the Virtual Machine Generation ID (vmgenid).
Virtual Machine Generation ID was introduced in commit af6b54e2b5ba
("virt: vmgenid: notify RNG of VM fork and supply generation ID") as an
ACPI only device.
VMGenID specification http://go.microsoft.com/fwlink/?LinkId=260709 defines
a mechanism for the BIOS/hypervisors to communicate to the virtual machine
that it is executed with a different configuration (e.g. snapshot execution
or creation from a template).
The guest operating system can use the notification for various purposes
such as re-initializing its random number generator etc.
More references to vmgenid specs:
- https://www.qemu.org/docs/master/specs/vmgenid.html
- https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/
virtual-machine-generation-identifier
*Reason for this change*:
Chosing ACPI or devicetree is an intrinsic part of an hypervisor design.
Without going into details of why a hypervisor would choose DT over ACPI,
we would like to highlight that the hypervisors that have chosen devicetree
and now want to make use of the vmgenid functionality cannot do so today
because vmgenid is an ACPI only device.
This forces these hypervisors to change their design which could have
undesirable impacts on their use-cases, test-scenarios etc.
vmgenid exposes to the guest a 16-byte cryptographically random number,
the value of which changes every time it starts executing from a new
configuration (snapshot, backup, etc.). During initialization, the device
exposes to the guest the address of the generation ID and
an interrupt number, which the device will use to notify the guest when
the generation ID changes.
These attributes can be trivially communicated via device tree bindings.
We believe that adding a devicetree binding for vmgenid is a simpler
alternative way to expose the device to the guest than forcing the
hypervisors to implement ACPI.
Addtional notes:
While adding the devicetree support we considered re-using existing
structures/code to avoid duplicating code and reduce maintenance; so,
we used the same driver to be configured either by ACPI or by DT.
This also meant reimplementing the existing vmgenid ACPI bus driver as a
platform driver and making it discoverable using `driver.of_match_table`
and `driver.acpi_match_table`.
There is no user impact or change in vmgenid functionality when used
with ACPI. We verified ACPI support of these patches on X86 and DT
support on ARM using Firecracker hypervisor
https://github.com/firecracker-microvm/firecracker.
To check schema and syntax errors, the bindings file is verified with:
```
make dt_binding_check \
DT_SCHEMA_FILES=\
Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
```
and the patches were verified with:
`scripts/checkpatch.pl --strict v4-000*`.
Changelog with respect to version 3:
- Changed the compatible string from "virtual,vmgenctr" to
"microsoft,vmgenid" as per review comments.
- Renamed vmgenid.yaml to follow DT file naming convention.
- Updated the description of properties and example in vmgenid yaml file.
- Addressed the review comments to remove all ifdefs in vmgenid.c with one
exception which still needs to be under CONFIG_ACPI.
- reformated the code with clang-format.
- Tested code with W=1, Sparse, Smatch and Coccinelle tools.
Changelog with respect to version 2:
- As per review comments, used platform apis instead of "of_*" APIs,
removed unnecessary #include and used IF_ENABLED instead of ifdef.
- Added more info for vmgenid buffer address and corrected the formatting.
- Replaced the compatible string from "linux,*" to "virtual,*" because,
the device does not have a vendor.
Changelog with respect to version 1:
- Moved vmgenid.yaml bindings to the more related "rng" folder.
- Removed `vmgenid_remove` to since it is unrelated to the
current goal of the patch.
- Updated the cover letter and bindings commit
"[PATCH v2 3/4] dt-bindings: rng: Add vmgenid support" to
provide more information on vmgenid.
- Compiled with and without CONFIG_OF/CONFIG_ACPI and fixed
compilers errors/warnings.
Sudan Landge (5):
virt: vmgenid: rearrange code to make review easier
virt: vmgenid: change implementation to use a platform driver
virt: vmgenid: enable driver regardless of ACPI config
dt-bindings: rng: Add vmgenid support
virt: vmgenid: add support for devicetree bindings
.../bindings/rng/microsoft,vmgenid.yaml | 49 +++++
MAINTAINERS | 1 +
drivers/virt/Kconfig | 1 -
drivers/virt/vmgenid.c | 168 ++++++++++++++----
4 files changed, 180 insertions(+), 39 deletions(-)
create mode 100644 Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
base-commit: 20cb38a7af88dc40095da7c2c9094da3873fea23
--
2.34.1
^ permalink raw reply
* [PATCH v4 1/5] virt: vmgenid: rearrange code to make review easier
From: Sudan Landge @ 2024-04-09 17:01 UTC (permalink / raw)
To: tytso, Jason, robh+dt, krzysztof.kozlowski+dt, conor+dt, sudanl,
sathyanarayanan.kuppuswamy, thomas.lendacky, dan.j.williams,
devicetree, linux-kernel
Cc: graf, dwmw, bchalios, xmarcalx
In-Reply-To: <20240409170137.79197-1-sudanl@amazon.com>
Rearrage the functions of vmgenid to make the next commit,
which re-implements vmgenid as a platform driver, easier to review.
Signed-off-by: Sudan Landge <sudanl@amazon.com>
---
drivers/virt/vmgenid.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index b67a28da4702..a167ccfad299 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -21,6 +21,20 @@ struct vmgenid_state {
u8 this_id[VMGENID_SIZE];
};
+static void vmgenid_notify(struct acpi_device *device, u32 event)
+{
+ struct vmgenid_state *state = acpi_driver_data(device);
+ char *envp[] = { "NEW_VMGENID=1", NULL };
+ u8 old_id[VMGENID_SIZE];
+
+ memcpy(old_id, state->this_id, sizeof(old_id));
+ memcpy(state->this_id, state->next_id, sizeof(state->this_id));
+ if (!memcmp(old_id, state->this_id, sizeof(old_id)))
+ return;
+ add_vmfork_randomness(state->this_id, sizeof(state->this_id));
+ kobject_uevent_env(&device->dev.kobj, KOBJ_CHANGE, envp);
+}
+
static int vmgenid_add(struct acpi_device *device)
{
struct acpi_buffer parsed = { ACPI_ALLOCATE_BUFFER };
@@ -65,25 +79,12 @@ static int vmgenid_add(struct acpi_device *device)
return ret;
}
-static void vmgenid_notify(struct acpi_device *device, u32 event)
-{
- struct vmgenid_state *state = acpi_driver_data(device);
- char *envp[] = { "NEW_VMGENID=1", NULL };
- u8 old_id[VMGENID_SIZE];
-
- memcpy(old_id, state->this_id, sizeof(old_id));
- memcpy(state->this_id, state->next_id, sizeof(state->this_id));
- if (!memcmp(old_id, state->this_id, sizeof(old_id)))
- return;
- add_vmfork_randomness(state->this_id, sizeof(state->this_id));
- kobject_uevent_env(&device->dev.kobj, KOBJ_CHANGE, envp);
-}
-
static const struct acpi_device_id vmgenid_ids[] = {
{ "VMGENCTR", 0 },
{ "VM_GEN_COUNTER", 0 },
{ }
};
+MODULE_DEVICE_TABLE(acpi, vmgenid_ids);
static struct acpi_driver vmgenid_driver = {
.name = "vmgenid",
@@ -97,7 +98,6 @@ static struct acpi_driver vmgenid_driver = {
module_acpi_driver(vmgenid_driver);
-MODULE_DEVICE_TABLE(acpi, vmgenid_ids);
MODULE_DESCRIPTION("Virtual Machine Generation ID");
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
--
2.34.1
^ permalink raw reply related
* [PATCH v4 2/5] virt: vmgenid: change implementation to use a platform driver
From: Sudan Landge @ 2024-04-09 17:01 UTC (permalink / raw)
To: tytso, Jason, robh+dt, krzysztof.kozlowski+dt, conor+dt, sudanl,
sathyanarayanan.kuppuswamy, thomas.lendacky, dan.j.williams,
devicetree, linux-kernel
Cc: graf, dwmw, bchalios, xmarcalx
In-Reply-To: <20240409170137.79197-1-sudanl@amazon.com>
Re-implement vmgenid as a platform driver in preparation
for adding devicetree bindings support in next commits.
Signed-off-by: Sudan Landge <sudanl@amazon.com>
---
drivers/virt/vmgenid.c | 97 +++++++++++++++++++++++++++++-------------
1 file changed, 67 insertions(+), 30 deletions(-)
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index a167ccfad299..c028e2064fdd 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -7,9 +7,10 @@
* information to random.c.
*/
+#include <linux/acpi.h>
#include <linux/kernel.h>
#include <linux/module.h>
-#include <linux/acpi.h>
+#include <linux/platform_device.h>
#include <linux/random.h>
ACPI_MODULE_NAME("vmgenid");
@@ -21,9 +22,9 @@ struct vmgenid_state {
u8 this_id[VMGENID_SIZE];
};
-static void vmgenid_notify(struct acpi_device *device, u32 event)
+static void vmgenid_notify(struct device *device)
{
- struct vmgenid_state *state = acpi_driver_data(device);
+ struct vmgenid_state *state = device->driver_data;
char *envp[] = { "NEW_VMGENID=1", NULL };
u8 old_id[VMGENID_SIZE];
@@ -32,21 +33,36 @@ static void vmgenid_notify(struct acpi_device *device, u32 event)
if (!memcmp(old_id, state->this_id, sizeof(old_id)))
return;
add_vmfork_randomness(state->this_id, sizeof(state->this_id));
- kobject_uevent_env(&device->dev.kobj, KOBJ_CHANGE, envp);
+ kobject_uevent_env(&device->kobj, KOBJ_CHANGE, envp);
+}
+
+static void vmgenid_acpi_handler(acpi_handle __always_unused handle,
+ u32 __always_unused event, void *dev)
+{
+ vmgenid_notify(dev);
+}
+
+static int setup_vmgenid_state(struct vmgenid_state *state, u8 *next_id)
+{
+ if (IS_ERR(next_id))
+ return PTR_ERR(next_id);
+
+ state->next_id = next_id;
+ memcpy(state->this_id, state->next_id, sizeof(state->this_id));
+ add_device_randomness(state->this_id, sizeof(state->this_id));
+ return 0;
}
-static int vmgenid_add(struct acpi_device *device)
+static int vmgenid_add_acpi(struct device *dev,
+ struct vmgenid_state *state)
{
+ struct acpi_device *device = ACPI_COMPANION(dev);
struct acpi_buffer parsed = { ACPI_ALLOCATE_BUFFER };
- struct vmgenid_state *state;
union acpi_object *obj;
phys_addr_t phys_addr;
acpi_status status;
int ret = 0;
-
- state = devm_kmalloc(&device->dev, sizeof(*state), GFP_KERNEL);
- if (!state)
- return -ENOMEM;
+ u8 *virt_addr;
status = acpi_evaluate_object(device->handle, "ADDR", NULL, &parsed);
if (ACPI_FAILURE(status)) {
@@ -63,40 +79,61 @@ static int vmgenid_add(struct acpi_device *device)
phys_addr = (obj->package.elements[0].integer.value << 0) |
(obj->package.elements[1].integer.value << 32);
- state->next_id = devm_memremap(&device->dev, phys_addr, VMGENID_SIZE, MEMREMAP_WB);
- if (IS_ERR(state->next_id)) {
- ret = PTR_ERR(state->next_id);
+
+ virt_addr = (u8 *)devm_memremap(&device->dev, phys_addr,
+ VMGENID_SIZE, MEMREMAP_WB);
+ ret = setup_vmgenid_state(state, virt_addr);
+ if (ret)
+ goto out;
+
+ dev->driver_data = state;
+ status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
+ vmgenid_acpi_handler, dev);
+ if (ACPI_FAILURE(status)) {
+ dev_err(dev, "Failed to install acpi notify handler");
+ ret = -ENODEV;
+ dev->driver_data = NULL;
goto out;
}
+out:
+ ACPI_FREE(parsed.pointer);
+ return ret;
+}
- memcpy(state->this_id, state->next_id, sizeof(state->this_id));
- add_device_randomness(state->this_id, sizeof(state->this_id));
+static int vmgenid_add(struct platform_device *pdev)
+{
+ struct vmgenid_state *state;
+ struct device *dev = &pdev->dev;
+ int ret = 0;
- device->driver_data = state;
+ state = devm_kmalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ ret = vmgenid_add_acpi(dev, state);
+
+ if (ret)
+ devm_kfree(dev, state);
-out:
- ACPI_FREE(parsed.pointer);
return ret;
}
-static const struct acpi_device_id vmgenid_ids[] = {
+static const struct acpi_device_id vmgenid_acpi_ids[] = {
{ "VMGENCTR", 0 },
{ "VM_GEN_COUNTER", 0 },
{ }
};
-MODULE_DEVICE_TABLE(acpi, vmgenid_ids);
-
-static struct acpi_driver vmgenid_driver = {
- .name = "vmgenid",
- .ids = vmgenid_ids,
- .owner = THIS_MODULE,
- .ops = {
- .add = vmgenid_add,
- .notify = vmgenid_notify
- }
+MODULE_DEVICE_TABLE(acpi, vmgenid_acpi_ids);
+
+static struct platform_driver vmgenid_plaform_driver = {
+ .probe = vmgenid_add,
+ .driver = {
+ .name = "vmgenid",
+ .acpi_match_table = vmgenid_acpi_ids,
+ },
};
-module_acpi_driver(vmgenid_driver);
+module_platform_driver(vmgenid_plaform_driver)
MODULE_DESCRIPTION("Virtual Machine Generation ID");
MODULE_LICENSE("GPL v2");
--
2.34.1
^ permalink raw reply related
* [PATCH v4 3/5] virt: vmgenid: enable driver regardless of ACPI config
From: Sudan Landge @ 2024-04-09 17:01 UTC (permalink / raw)
To: tytso, Jason, robh+dt, krzysztof.kozlowski+dt, conor+dt, sudanl,
sathyanarayanan.kuppuswamy, thomas.lendacky, dan.j.williams,
devicetree, linux-kernel
Cc: graf, dwmw, bchalios, xmarcalx
In-Reply-To: <20240409170137.79197-1-sudanl@amazon.com>
Since with next commits vmgenid driver will support both ACPI and
devicetree, and since either one of CONFIG_ACPI or CONFIG_OF will
always be enabled, there is no need for the driver compilation to
depend on ACPI/OF. So, remove ACPI dependency for compiling the driver.
Signed-off-by: Sudan Landge <sudanl@amazon.com>
---
drivers/virt/Kconfig | 1 -
drivers/virt/vmgenid.c | 16 +++++++++++-----
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 40129b6f0eca..d8c848cf09a6 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -16,7 +16,6 @@ if VIRT_DRIVERS
config VMGENID
tristate "Virtual Machine Generation ID driver"
default y
- depends on ACPI
help
Say Y here to use the hypervisor-provided Virtual Machine Generation ID
to reseed the RNG when the VM is cloned. This is highly recommended if
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index c028e2064fdd..3d93e3fb94c4 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -36,13 +36,15 @@ static void vmgenid_notify(struct device *device)
kobject_uevent_env(&device->kobj, KOBJ_CHANGE, envp);
}
-static void vmgenid_acpi_handler(acpi_handle __always_unused handle,
- u32 __always_unused event, void *dev)
+static void __maybe_unused
+vmgenid_acpi_handler(acpi_handle __always_unused handle,
+ u32 __always_unused event, void *dev)
{
vmgenid_notify(dev);
}
-static int setup_vmgenid_state(struct vmgenid_state *state, u8 *next_id)
+static int __maybe_unused
+setup_vmgenid_state(struct vmgenid_state *state, u8 *next_id)
{
if (IS_ERR(next_id))
return PTR_ERR(next_id);
@@ -53,9 +55,10 @@ static int setup_vmgenid_state(struct vmgenid_state *state, u8 *next_id)
return 0;
}
-static int vmgenid_add_acpi(struct device *dev,
- struct vmgenid_state *state)
+static int vmgenid_add_acpi(struct device __maybe_unused *dev,
+ struct vmgenid_state __maybe_unused *state)
{
+#if IS_ENABLED(CONFIG_ACPI)
struct acpi_device *device = ACPI_COMPANION(dev);
struct acpi_buffer parsed = { ACPI_ALLOCATE_BUFFER };
union acpi_object *obj;
@@ -98,6 +101,9 @@ static int vmgenid_add_acpi(struct device *dev,
out:
ACPI_FREE(parsed.pointer);
return ret;
+#else
+ return -EINVAL;
+#endif
}
static int vmgenid_add(struct platform_device *pdev)
--
2.34.1
^ permalink raw reply related
* [PATCH v4 4/5] dt-bindings: rng: Add vmgenid support
From: Sudan Landge @ 2024-04-09 17:01 UTC (permalink / raw)
To: tytso, Jason, robh+dt, krzysztof.kozlowski+dt, conor+dt, sudanl,
sathyanarayanan.kuppuswamy, thomas.lendacky, dan.j.williams,
devicetree, linux-kernel
Cc: graf, dwmw, bchalios, xmarcalx
In-Reply-To: <20240409170137.79197-1-sudanl@amazon.com>
Virtual Machine Generation ID driver was introduced in commit af6b54e2b5ba
("virt: vmgenid: notify RNG of VM fork and supply generation ID"), as an
ACPI only device.
VMGenID specification http://go.microsoft.com/fwlink/?LinkId=260709 defines
a mechanism for the BIOS/hypervisors to communicate to the virtual machine
that it is executed with a different configuration (e.g. snapshot execution
or creation from a template).
The guest operating system can use the notification for various purposes
such as re-initializing its random number generator etc.
As per the specs, hypervisor should provide a globally unique identified,
or GUID via ACPI.
This patch tries to mimic the mechanism to provide the same functionality
which is for a hypervisor/BIOS to notify the virtual machine when it is
executed with a different configuration.
As part of this support the devicetree bindings requires the hypervisors or
BIOS to provide a memory address which holds the GUID and an IRQ which is
used to notify when there is a change in the GUID.
The memory exposed in the DT should follow the rules defined in the
vmgenid spec mentioned above.
*Reason for this change*:
Chosing ACPI or devicetree is an intrinsic part of an hypervisor design.
Without going into details of why a hypervisor would choose DT over ACPI,
we would like to highlight that the hypervisors that have chose devicetree
and now want to make use of the vmgenid functionality cannot do so today
because vmgenid is an ACPI only device.
This forces these hypervisors to change their design which could have
undesirable impacts on their use-cases, test-scenarios etc.
vmgenid exposes to the guest a 16-byte cryptographically random number,
the value of which changes every time it starts executing from a new
configuration (snapshot, backup, etc.). During initialization, the device
exposes to the guest the address of the generation ID and
an interrupt number, which the device will use to notify the guest when
the generation ID changes.
These attributes can be trivially communicated via device tree bindings.
We believe that adding a devicetree binding for vmgenid is a simpler
alternative way to expose the device to the guest than forcing the
hypervisors to implement ACPI.
More references to vmgenid specs:
- https://www.qemu.org/docs/master/specs/vmgenid.html
- https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/virtual-
machine-generation-identifier
Signed-off-by: Sudan Landge <sudanl@amazon.com>
---
.../bindings/rng/microsoft,vmgenid.yaml | 49 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
diff --git a/Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml b/Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
new file mode 100644
index 000000000000..8f20dee93e7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rng/microsoft,vmgenid.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtual Machine Generation ID
+
+maintainers:
+ - Jason A. Donenfeld <Jason@zx2c4.com>
+
+description:
+ Firmwares or hypervisors can use this devicetree to describe an
+ interrupt and a shared resource to inject a Virtual Machine Generation ID.
+ Virtual Machine Generation ID is a globally unique identifier (GUID) and
+ the devicetree binding follows VMGenID specification defined in
+ http://go.microsoft.com/fwlink/?LinkId=260709.
+
+properties:
+ compatible:
+ const: microsoft,vmgenid
+
+ reg:
+ description:
+ Specifies a 16-byte VMGenID in endianness-agnostic hexadecimal format.
+ maxItems: 1
+
+ interrupts:
+ description:
+ Interrupt used to notify that a new VMGenID is available.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ rng@80000000 {
+ compatible = "microsoft,vmgenid";
+ reg = <0x80000000 0x1000>;
+ interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>;
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index aea47e04c3a5..243607744b7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18476,6 +18476,7 @@ M: "Theodore Ts'o" <tytso@mit.edu>
M: Jason A. Donenfeld <Jason@zx2c4.com>
S: Maintained
T: git https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git
+F: Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
F: drivers/char/random.c
F: drivers/virt/vmgenid.c
--
2.34.1
^ permalink raw reply related
* [PATCH v4 5/5] virt: vmgenid: add support for devicetree bindings
From: Sudan Landge @ 2024-04-09 17:07 UTC (permalink / raw)
To: tytso, Jason, robh+dt, krzysztof.kozlowski+dt, conor+dt, sudanl,
sathyanarayanan.kuppuswamy, thomas.lendacky, dan.j.williams,
devicetree, linux-kernel
Cc: graf, dwmw, bchalios, xmarcalx
Extend the vmgenid platform driver to support devicetree bindings.
With this support, hypervisors can send vmgenid notifications to
the virtual machine without the need to enable ACPI.
The bindings are located at:
Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
Signed-off-by: Sudan Landge <sudanl@amazon.com>
---
drivers/virt/vmgenid.c | 53 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 51 insertions(+), 2 deletions(-)
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index 3d93e3fb94c4..e1ad74116c0c 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -2,12 +2,13 @@
/*
* Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
*
- * The "Virtual Machine Generation ID" is exposed via ACPI and changes when a
+ * The "Virtual Machine Generation ID" is exposed via ACPI or DT and changes when a
* virtual machine forks or is cloned. This driver exists for shepherding that
* information to random.c.
*/
#include <linux/acpi.h>
+#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -20,6 +21,7 @@ enum { VMGENID_SIZE = 16 };
struct vmgenid_state {
u8 *next_id;
u8 this_id[VMGENID_SIZE];
+ int irq;
};
static void vmgenid_notify(struct device *device)
@@ -43,6 +45,14 @@ vmgenid_acpi_handler(acpi_handle __always_unused handle,
vmgenid_notify(dev);
}
+static __maybe_unused irqreturn_t
+vmgenid_of_irq_handler(int __always_unused irq, void *dev)
+{
+ vmgenid_notify(dev);
+
+ return IRQ_HANDLED;
+}
+
static int __maybe_unused
setup_vmgenid_state(struct vmgenid_state *state, u8 *next_id)
{
@@ -106,6 +116,35 @@ static int vmgenid_add_acpi(struct device __maybe_unused *dev,
#endif
}
+static int vmgenid_add_of(struct platform_device *pdev,
+ struct vmgenid_state *state)
+{
+ u8 *virt_addr;
+ int ret = 0;
+
+ virt_addr = (u8 *)devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+ if (IS_ERR(virt_addr))
+ return PTR_ERR(virt_addr);
+
+ ret = setup_vmgenid_state(state, virt_addr);
+ if (ret)
+ return ret;
+
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0)
+ return ret;
+
+ state->irq = ret;
+ pdev->dev.driver_data = state;
+
+ ret = devm_request_irq(&pdev->dev, state->irq, vmgenid_of_irq_handler,
+ IRQF_SHARED, "vmgenid", &pdev->dev);
+ if (ret)
+ pdev->dev.driver_data = NULL;
+
+ return ret;
+}
+
static int vmgenid_add(struct platform_device *pdev)
{
struct vmgenid_state *state;
@@ -116,7 +155,10 @@ static int vmgenid_add(struct platform_device *pdev)
if (!state)
return -ENOMEM;
- ret = vmgenid_add_acpi(dev, state);
+ if (dev->of_node)
+ ret = vmgenid_add_of(pdev, state);
+ else
+ ret = vmgenid_add_acpi(dev, state);
if (ret)
devm_kfree(dev, state);
@@ -124,6 +166,12 @@ static int vmgenid_add(struct platform_device *pdev)
return ret;
}
+static const struct of_device_id vmgenid_of_ids[] = {
+ { .compatible = "microsoft,vmgenid", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, vmgenid_of_ids);
+
static const struct acpi_device_id vmgenid_acpi_ids[] = {
{ "VMGENCTR", 0 },
{ "VM_GEN_COUNTER", 0 },
@@ -136,6 +184,7 @@ static struct platform_driver vmgenid_plaform_driver = {
.driver = {
.name = "vmgenid",
.acpi_match_table = vmgenid_acpi_ids,
+ .of_match_table = vmgenid_of_ids,
},
};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net-next v7 11/17] dt-bindings: net: pse-pd: Add another way of describing several PSE PIs
From: Rob Herring @ 2024-04-09 17:08 UTC (permalink / raw)
To: Kory Maincent
Cc: Frank Rowand, netdev, Andrew Lunn, Jakub Kicinski, linux-doc,
Dent Project, Heiner Kallweit, Oleksij Rempel, Thomas Petazzoni,
linux-kernel, kernel, Russell King, Paolo Abeni, Luis Chamberlain,
Conor Dooley, Jonathan Corbet, devicetree, Russ Weight,
Maxime Chevallier, Rafael J. Wysocki, Eric Dumazet,
David S. Miller, Krzysztof Kozlowski, Mark Brown,
Greg Kroah-Hartman, Rob Herring
In-Reply-To: <20240409-feature_poe-v7-11-11e38efd4dee@bootlin.com>
On Tue, 09 Apr 2024 17:04:01 +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
>
> PSE PI setup may encompass multiple PSE controllers or auxiliary circuits
> that collectively manage power delivery to one Ethernet port.
> Such configurations might support a range of PoE standards and require
> the capability to dynamically configure power delivery based on the
> operational mode (e.g., PoE2 versus PoE4) or specific requirements of
> connected devices. In these instances, a dedicated PSE PI node becomes
> essential for accurately documenting the system architecture. This node
> would serve to detail the interactions between different PSE controllers,
> the support for various PoE modes, and any additional logic required to
> coordinate power delivery across the network infrastructure.
>
> The old usage of "#pse-cells" is unsuficient as it carries only the PSE PI
> index information.
>
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>
> Changes in v3:
> - New patch
>
> Changes in v4:
> - Remove $def
> - Fix pairset-names item list
> - Upgrade few properties description
> - Update the commit message
>
> Changes in v5:
> - Fix yamllint error.
> - Replace underscore by dash in properties names.
> - Add polarity-supported property.
>
> Changes in v6:
> - Reorder the pairset pinout table documentation to shrink the lines size.
> - Remove pairset and polarity as required fields.
> - Add vpwr-supply regulator supply.
>
> Changes in v7:
> - Fix weird characters issue.
> - Fix documentation nit.
> ---
> .../bindings/net/pse-pd/pse-controller.yaml | 101 ++++++++++++++++++++-
> 1 file changed, 98 insertions(+), 3 deletions(-)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
Warning: Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml references a file that doesn't exist: Documentation/networking/pse-pd/pse-pi.rst
Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml: Documentation/networking/pse-pd/pse-pi.rst
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240409-feature_poe-v7-11-11e38efd4dee@bootlin.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH] arm64: dts: ti: k3-am625-phyboard-lyra-rdk: Add Audio Codec
From: Wadim Egorov @ 2024-04-09 17:10 UTC (permalink / raw)
To: Garrett Giordano, nm, vigneshr, kristo, robh,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-kernel, devicetree, linux-kernel, upstream
In-Reply-To: <20240404184250.3772829-1-ggiordano@phytec.com>
Am 04.04.24 um 20:42 schrieb Garrett Giordano:
> The Audio Codec runs over the MCASP (Multichannel Audio Serial Port).
>
> Add pinmux for the Audio Reference Clock and MCASP2.
>
> Add DT nodes for Audio Codec, MCASP2, VCC 1v8 and VCC 3v3 regulators.
>
> Additionally, create a sound node that connects our sound card and the
> MCASP2.
>
> Signed-off-by: Garrett Giordano <ggiordano@phytec.com>
Reviewed-by: Wadim Egorov <w.egorov@phytec.de>
> ---
> .../dts/ti/k3-am625-phyboard-lyra-rdk.dts | 99 +++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts
> index a83a90497857..dfc78995d30a 100644
> --- a/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts
> @@ -66,6 +66,35 @@ key-menu {
> };
> };
>
> + sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "phyBOARD-Lyra";
> + simple-audio-card,widgets =
> + "Microphone", "Mic Jack",
> + "Headphone", "Headphone Jack",
> + "Speaker", "External Speaker";
> + simple-audio-card,routing =
> + "MIC3R", "Mic Jack",
> + "Mic Jack", "Mic Bias",
> + "Headphone Jack", "HPLOUT",
> + "Headphone Jack", "HPROUT",
> + "External Speaker", "SPOP",
> + "External Speaker", "SPOM";
> + simple-audio-card,format = "dsp_b";
> + simple-audio-card,bitclock-master = <&sound_master>;
> + simple-audio-card,frame-master = <&sound_master>;
> + simple-audio-card,bitclock-inversion;
> +
> + simple-audio-card,cpu {
> + sound-dai = <&mcasp2>;
> + };
> +
> + sound_master: simple-audio-card,codec {
> + sound-dai = <&audio_codec>;
> + clocks = <&audio_refclk1>;
> + };
> + };
> +
> leds {
> compatible = "gpio-leds";
> pinctrl-names = "default";
> @@ -82,6 +111,15 @@ led-2 {
> };
> };
>
> + vcc_1v8: regulator-vcc-1v8 {
> + compatible = "regulator-fixed";
> + regulator-name = "VCC_1V8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> vcc_3v3_mmc: regulator-vcc-3v3-mmc {
> compatible = "regulator-fixed";
> regulator-name = "VCC_3V3_MMC";
> @@ -90,9 +128,24 @@ vcc_3v3_mmc: regulator-vcc-3v3-mmc {
> regulator-always-on;
> regulator-boot-on;
> };
> +
> + vcc_3v3_sw: regulator-vcc-3v3-sw {
> + compatible = "regulator-fixed";
> + regulator-name = "VCC_3V3_SW";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> };
>
> &main_pmx0 {
> + audio_ext_refclk1_pins_default: audio-ext-refclk1-default-pins {
> + pinctrl-single,pins = <
> + AM62X_IOPAD(0x0a0, PIN_OUTPUT, 1) /* (K25) GPMC0_WPn.AUDIO_EXT_REFCLK1 */
> + >;
> + };
> +
> gpio_keys_pins_default: gpio-keys-default-pins {
> pinctrl-single,pins = <
> AM62X_IOPAD(0x1d4, PIN_INPUT, 7) /* (B15) UART0_RTSn.GPIO1_23 */
> @@ -150,6 +203,15 @@ AM62X_IOPAD(0x1d8, PIN_OUTPUT, 0) /* (C15) MCAN0_TX */
> >;
> };
>
> + main_mcasp2_pins_default: main-mcasp2-default-pins {
> + pinctrl-single,pins = <
> + AM62X_IOPAD(0x070, PIN_INPUT, 3) /* (T24) GPMC0_AD13.MCASP2_ACLKX */
> + AM62X_IOPAD(0x06c, PIN_INPUT, 3) /* (T22) GPMC0_AD12.MCASP2_AFSX */
> + AM62X_IOPAD(0x064, PIN_OUTPUT, 3) /* (T25) GPMC0_AD10.MCASP2_AXR2 */
> + AM62X_IOPAD(0x068, PIN_INPUT, 3) /* (R21) GPMC0_AD11.MCASP2_AXR3 */
> + >;
> + };
> +
> main_mmc1_pins_default: main-mmc1-default-pins {
> pinctrl-single,pins = <
> AM62X_IOPAD(0x23c, PIN_INPUT_PULLUP, 0) /* (A21) MMC1_CMD */
> @@ -254,6 +316,21 @@ &main_i2c1 {
> clock-frequency = <100000>;
> status = "okay";
>
> + audio_codec: audio-codec@18 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&audio_ext_refclk1_pins_default>;
> +
> + #sound-dai-cells = <0>;
> + compatible = "ti,tlv320aic3007";
> + reg = <0x18>;
> + ai3x-micbias-vg = <2>;
> +
> + AVDD-supply = <&vcc_3v3_sw>;
> + IOVDD-supply = <&vcc_3v3_sw>;
> + DRVDD-supply = <&vcc_3v3_sw>;
> + DVDD-supply = <&vcc_1v8>;
> + };
> +
> gpio_exp: gpio-expander@21 {
> pinctrl-names = "default";
> pinctrl-0 = <&gpio_exp_int_pins_default>;
> @@ -329,6 +406,28 @@ &main_uart1 {
> status = "okay";
> };
>
> +&mcasp2 {
> + status = "okay";
> + #sound-dai-cells = <0>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&main_mcasp2_pins_default>;
> +
> + /* MCASP_IIS_MODE */
> + op-mode = <0>;
> + tdm-slots = <2>;
> +
> + /* 0: INACTIVE, 1: TX, 2: RX */
> + serial-dir = <
> + 0 0 1 2
> + 0 0 0 0
> + 0 0 0 0
> + 0 0 0 0
> + >;
> + tx-num-evt = <32>;
> + rx-num-evt = <32>;
> +};
> +
> &sdhci1 {
> vmmc-supply = <&vcc_3v3_mmc>;
> vqmmc-supply = <&vddshv5_sdio>;
^ permalink raw reply
* Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value
From: Rob Clark @ 2024-04-09 17:12 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Konrad Dybcio, Bjorn Andersson, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <tkcbl2iwcy3feoggbk737gx32qsxe5p4ad6lfrujy2pazccrhm@kif3guuzay6h>
On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
> >
> >
> > On 4/6/24 04:56, Dmitry Baryshkov wrote:
> > > On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> > > > From: Neil Armstrong <neil.armstrong@linaro.org>
> > > >
> > > > Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> > > > the highest. Falling back to it when things go wrong is largely
> > > > suboptimal, as more often than not, the top frequencies are not
> > > > supposed to work on other bins.
> > >
> > > Isn't it better to just return an error here instead of trying to guess
> > > which speedbin to use?
> >
> > Not sure. I'd rather better compatibility for e.g. booting up a new
> > laptop with just dt.
>
> New speedbin can have lower max speed, so by attempting to run it at
> higher freq you might be breaking it.
Usually there are some OPPs in common to all speedbins, so picking a
freq from that set would seem like the safe thing to do
BR,
-R
>
> >
> > >
> > > If that's not the case, I think the commit should be expanded with
> > > actually setting default_speedbin for the existing GPUs.
> >
> > I think that should be addressed, although separately.
>
> I'd prefer to have it as a part of this patch, but I'd not NAK it just
> for this reason.
>
> --
> With best wishes
> Dmitry
^ permalink raw reply
* Re: [PATCH v9 00/13] Introduce STM32 Firewall framework
From: Rob Herring @ 2024-04-09 17:13 UTC (permalink / raw)
To: Alexandre TORGUE
Cc: Gatien Chevallier, Oleksii_Moisieiev, gregkh, herbert, davem,
krzysztof.kozlowski+dt, conor+dt, vkoul, jic23, olivier.moysan,
arnaud.pouliquen, mchehab, fabrice.gasnier, andi.shyti,
ulf.hansson, edumazet, kuba, pabeni, hugues.fruchet, lee, will,
catalin.marinas, arnd, richardcochran, Frank Rowand, peng.fan,
lars, rcsekar, wg, mkl, linux-crypto, devicetree, linux-stm32,
linux-arm-kernel, linux-kernel, dmaengine, linux-i2c, linux-iio,
alsa-devel, linux-media, linux-mmc, netdev, linux-phy,
linux-serial, linux-spi, linux-usb
In-Reply-To: <61608010-fbce-46c6-a83d-94c04d0f000d@foss.st.com>
On Mon, Apr 8, 2024 at 3:44 AM Alexandre TORGUE
<alexandre.torgue@foss.st.com> wrote:
>
> Hi Gatien,
>
> On 1/5/24 14:03, Gatien Chevallier wrote:
> > Introduce STM32 Firewall framework for STM32MP1x and STM32MP2x
> > platforms. STM32MP1x(ETZPC) and STM32MP2x(RIFSC) Firewall controllers
> > register to the framework to offer firewall services such as access
> > granting.
> >
> > This series of patches is a new approach on the previous STM32 system
> > bus, history is available here:
> > https://lore.kernel.org/lkml/20230127164040.1047583/
> >
> > The need for such framework arises from the fact that there are now
> > multiple hardware firewalls implemented across multiple products.
> > Drivers are shared between different products, using the same code.
> > When it comes to firewalls, the purpose mostly stays the same: Protect
> > hardware resources. But the implementation differs, and there are
> > multiple types of firewalls: peripheral, memory, ...
> >
> > Some hardware firewall controllers such as the RIFSC implemented on
> > STM32MP2x platforms may require to take ownership of a resource before
> > being able to use it, hence the requirement for firewall services to
> > take/release the ownership of such resources.
> >
> > On the other hand, hardware firewall configurations are becoming
> > more and more complex. These mecanisms prevent platform crashes
> > or other firewall-related incoveniences by denying access to some
> > resources.
> >
> > The stm32 firewall framework offers an API that is defined in
> > firewall controllers drivers to best fit the specificity of each
> > firewall.
> >
> > For every peripherals protected by either the ETZPC or the RIFSC, the
> > firewall framework checks the firewall controlelr registers to see if
> > the peripheral's access is granted to the Linux kernel. If not, the
> > peripheral is configured as secure, the node is marked populated,
> > so that the driver is not probed for that device.
> >
> > The firewall framework relies on the access-controller device tree
> > binding. It is used by peripherals to reference a domain access
> > controller. In this case a firewall controller. The bus uses the ID
> > referenced by the access-controller property to know where to look
> > in the firewall to get the security configuration for the peripheral.
> > This allows a device tree description rather than a hardcoded peripheral
> > table in the bus driver.
> >
> > The STM32 ETZPC device is responsible for filtering accesses based on
> > security level, or co-processor isolation for any resource connected
> > to it.
> >
> > The RIFSC is responsible for filtering accesses based on Compartment
> > ID / security level / privilege level for any resource connected to
> > it.
> >
> > STM32MP13/15/25 SoC device tree files are updated in this series to
> > implement this mecanism.
> >
>
> ...
>
> After minor cosmetic fixes, series applied on stm32-next.
> Seen with Arnd: it will be part on my next PR and will come through
> arm-soc tree.
And there's some new warnings in next with it:
1 venc@480e0000: 'access-controllers' does not match any of the
regexes: 'pinctrl-[0-9]+'
1 vdec@480d0000: 'access-controllers' does not match any of the
regexes: 'pinctrl-[0-9]+'
Rob
^ permalink raw reply
* [PATCH v10 0/4] Introduce STM32MP257 clock driver
From: gabriel.fernandez @ 2024-04-09 17:12 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Maxime Coquelin, Alexandre Torgue, Philipp Zabel,
Gabriel Fernandez
Cc: linux-clk, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel
From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
v10:
- patch 'dt-bindings: stm32: update DT bingding for stm32mp25'
- cosmetic change (add change blank line) to YAML documentation
- rename USB2 into USBH to be compliant with last Reference Manuel
- Update commit message
cant't add 'Acked-by: Conor Dooley <conor.dooley@microchip.com>' because modification above
- patch 'clk: stm32: introduce clocks for STM32MP257 platform'
- rename USB2 into USBH to be compliant with last Reference Manuel
- cosmetic changes to be aligned with open parenthesis.
v9: base on next-20240207
- update dt binding documentation with v8 modidification on RCC driver
(use .index of clk_parent_data struct to define a parent)
- rebase patch "arm64: dts: st: add rcc support for STM32MP25"
with next-20240207 tag
v8:
- use .index of clk_parent_data struct to define a parent
- remove unnecessary dependency check with SCMI clock driver
- convert to platform device APIs
- convert to devm_of_clk_add_hw_provider()
- convert single value enum to a define
v7: base on next-20231219
- These patches below are applied to clk-next:
clk: stm32mp1: move stm32mp1 clock driver into stm32 directory
clk: stm32mp1: use stm32mp13 reset driver
dt-bindings: stm32: add clocks and reset binding for stm32mp25
- remove unnecessary includes
- migrate clock parents to struct clk_parent_data and remove
CLK_STM32_XXX() macros to have a more readble code
- use platform device APIs (devm_of_iomap() instead of_iomap())
- move content of stm32mp25_rcc_init() to stm32mp25_rcc_clocks_probe()
- simply get_clock_deps()
- add const to stm32mp25_data struct
- remove ck_icn_p_serc clock (will be integrate later with security
management)
v6:
- remove useless defines in drivers/clk/stm32/stm32mp25_rcc.h
v5:
- Fix sparse warnings: was not declared. Should it be static?
drivers/clk/stm32/clk-stm32mp13.c:1516:29: symbol 'stm32mp13_reset_data'
drivers/clk/stm32/clk-stm32mp1.c:2148:29: symbol 'stm32mp1_reset_data'
drivers/clk/stm32/clk-stm32mp25.c:1003:5: symbol 'stm32mp25_cpt_gate'
drivers/clk/stm32/clk-stm32mp25.c:1005:29: symbol 'stm32mp25_clock_data'
drivers/clk/stm32/clk-stm32mp25.c:1011:29: symbol 'stm32mp25_reset_data'
v4:
- use GPL-2.0-only OR BSD-2-Clause for clock and reset binding files
- use quotes ' for #clock-cells and #reset-cells in YAML documentation
- reset binding start now to 0 instead 1
- improve management of reset lines that are not managed
v3:
- from Rob Herring change clock item description in YAML documentation
v2:
- rework reset binding (use ID witch start from 0)
- rework reset driver to manage STM32MP13 / STM32MP15 / STM32MP25
- rework YAML documentation
Gabriel Fernandez (4):
clk: stm32mp13: use platform device APIs
dt-bindings: stm32: update DT bingding for stm32mp25
clk: stm32: introduce clocks for STM32MP257 platform
arm64: dts: st: add rcc support for STM32MP25
.../bindings/clock/st,stm32mp25-rcc.yaml | 170 +-
arch/arm64/boot/dts/st/stm32mp251.dtsi | 144 +-
arch/arm64/boot/dts/st/stm32mp255.dtsi | 4 +-
drivers/clk/stm32/Kconfig | 7 +
drivers/clk/stm32/Makefile | 1 +
drivers/clk/stm32/clk-stm32-core.c | 11 +-
drivers/clk/stm32/clk-stm32mp13.c | 72 +-
drivers/clk/stm32/clk-stm32mp25.c | 1876 +++++++++++++++++
drivers/clk/stm32/reset-stm32.c | 59 +-
drivers/clk/stm32/reset-stm32.h | 7 +
drivers/clk/stm32/stm32mp25_rcc.h | 712 +++++++
include/dt-bindings/reset/st,stm32mp25-rcc.h | 2 +-
12 files changed, 2923 insertions(+), 142 deletions(-)
create mode 100644 drivers/clk/stm32/clk-stm32mp25.c
create mode 100644 drivers/clk/stm32/stm32mp25_rcc.h
--
2.25.1
^ permalink raw reply
* [PATCH v10 1/4] clk: stm32mp13: use platform device APIs
From: gabriel.fernandez @ 2024-04-09 17:12 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Maxime Coquelin, Alexandre Torgue, Philipp Zabel,
Gabriel Fernandez
Cc: linux-clk, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <20240409171241.274600-1-gabriel.fernandez@foss.st.com>
From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
Convert devm_platform_ioremap_resource() and remove unnecessary
dependency check with SCMI clock driver.
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
---
drivers/clk/stm32/clk-stm32-core.c | 11 +++--
drivers/clk/stm32/clk-stm32mp13.c | 72 +++---------------------------
2 files changed, 10 insertions(+), 73 deletions(-)
diff --git a/drivers/clk/stm32/clk-stm32-core.c b/drivers/clk/stm32/clk-stm32-core.c
index 58705fcad334..1721a3ed7386 100644
--- a/drivers/clk/stm32/clk-stm32-core.c
+++ b/drivers/clk/stm32/clk-stm32-core.c
@@ -25,7 +25,6 @@ static int stm32_rcc_clock_init(struct device *dev,
{
const struct stm32_rcc_match_data *data = match->data;
struct clk_hw_onecell_data *clk_data = data->hw_clks;
- struct device_node *np = dev_of_node(dev);
struct clk_hw **hws;
int n, max_binding;
@@ -64,7 +63,7 @@ static int stm32_rcc_clock_init(struct device *dev,
hws[cfg_clock->id] = hw;
}
- return of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
+ return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
}
int stm32_rcc_init(struct device *dev, const struct of_device_id *match_data,
@@ -638,7 +637,7 @@ struct clk_hw *clk_stm32_mux_register(struct device *dev,
mux->lock = lock;
mux->clock_data = data->clock_data;
- err = clk_hw_register(dev, hw);
+ err = devm_clk_hw_register(dev, hw);
if (err)
return ERR_PTR(err);
@@ -659,7 +658,7 @@ struct clk_hw *clk_stm32_gate_register(struct device *dev,
gate->lock = lock;
gate->clock_data = data->clock_data;
- err = clk_hw_register(dev, hw);
+ err = devm_clk_hw_register(dev, hw);
if (err)
return ERR_PTR(err);
@@ -680,7 +679,7 @@ struct clk_hw *clk_stm32_div_register(struct device *dev,
div->lock = lock;
div->clock_data = data->clock_data;
- err = clk_hw_register(dev, hw);
+ err = devm_clk_hw_register(dev, hw);
if (err)
return ERR_PTR(err);
@@ -701,7 +700,7 @@ struct clk_hw *clk_stm32_composite_register(struct device *dev,
composite->lock = lock;
composite->clock_data = data->clock_data;
- err = clk_hw_register(dev, hw);
+ err = devm_clk_hw_register(dev, hw);
if (err)
return ERR_PTR(err);
diff --git a/drivers/clk/stm32/clk-stm32mp13.c b/drivers/clk/stm32/clk-stm32mp13.c
index d4ecb3c34a1b..bf81d7491708 100644
--- a/drivers/clk/stm32/clk-stm32mp13.c
+++ b/drivers/clk/stm32/clk-stm32mp13.c
@@ -1536,77 +1536,16 @@ static const struct of_device_id stm32mp13_match_data[] = {
};
MODULE_DEVICE_TABLE(of, stm32mp13_match_data);
-static int stm32mp1_rcc_init(struct device *dev)
-{
- void __iomem *rcc_base;
- int ret = -ENOMEM;
-
- rcc_base = of_iomap(dev_of_node(dev), 0);
- if (!rcc_base) {
- dev_err(dev, "%pOFn: unable to map resource", dev_of_node(dev));
- goto out;
- }
-
- ret = stm32_rcc_init(dev, stm32mp13_match_data, rcc_base);
-out:
- if (ret) {
- if (rcc_base)
- iounmap(rcc_base);
-
- of_node_put(dev_of_node(dev));
- }
-
- return ret;
-}
-
-static int get_clock_deps(struct device *dev)
-{
- static const char * const clock_deps_name[] = {
- "hsi", "hse", "csi", "lsi", "lse",
- };
- size_t deps_size = sizeof(struct clk *) * ARRAY_SIZE(clock_deps_name);
- struct clk **clk_deps;
- int i;
-
- clk_deps = devm_kzalloc(dev, deps_size, GFP_KERNEL);
- if (!clk_deps)
- return -ENOMEM;
-
- for (i = 0; i < ARRAY_SIZE(clock_deps_name); i++) {
- struct clk *clk = of_clk_get_by_name(dev_of_node(dev),
- clock_deps_name[i]);
-
- if (IS_ERR(clk)) {
- if (PTR_ERR(clk) != -EINVAL && PTR_ERR(clk) != -ENOENT)
- return PTR_ERR(clk);
- } else {
- /* Device gets a reference count on the clock */
- clk_deps[i] = devm_clk_get(dev, __clk_get_name(clk));
- clk_put(clk);
- }
- }
-
- return 0;
-}
-
static int stm32mp1_rcc_clocks_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- int ret = get_clock_deps(dev);
+ void __iomem *base;
- if (!ret)
- ret = stm32mp1_rcc_init(dev);
-
- return ret;
-}
-
-static void stm32mp1_rcc_clocks_remove(struct platform_device *pdev)
-{
- struct device *dev = &pdev->dev;
- struct device_node *child, *np = dev_of_node(dev);
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (WARN_ON(IS_ERR(base)))
+ return PTR_ERR(base);
- for_each_available_child_of_node(np, child)
- of_clk_del_provider(child);
+ return stm32_rcc_init(dev, stm32mp13_match_data, base);
}
static struct platform_driver stm32mp13_rcc_clocks_driver = {
@@ -1615,7 +1554,6 @@ static struct platform_driver stm32mp13_rcc_clocks_driver = {
.of_match_table = stm32mp13_match_data,
},
.probe = stm32mp1_rcc_clocks_probe,
- .remove_new = stm32mp1_rcc_clocks_remove,
};
static int __init stm32mp13_clocks_init(void)
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox