* Re: [PATCH v4 1/7] iio: accel: adxl345: Make data_range obsolete
From: Jonathan Cameron @ 2024-03-25 20:31 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-iio, devicetree, linux-kernel, eraretuya
In-Reply-To: <20240325153356.46112-2-l.rubusch@gmail.com>
On Mon, 25 Mar 2024 15:33:50 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:
> Replace write() data_format by regmap_update_bits(), because
> bus specific pre-configuration may have happened before on
> the same register. Changes then need to be masked.
>
> Remove the data_range field from the struct adxl345_data,
> because it is not used anymore.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
> drivers/iio/accel/adxl345_core.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 8bd30a23e..be6758015 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -42,13 +42,13 @@
> #define ADXL345_DATA_FORMAT_4G 1
> #define ADXL345_DATA_FORMAT_8G 2
> #define ADXL345_DATA_FORMAT_16G 3
> +#define ADXL345_DATA_FORMAT_MSK ~((u8) BIT(6)) /* ignore spi-3wire */
I'm not keen on seeing masking of a bit we don't yet
handle done by value. Can we instead build this up by what we 'want' to
write rather than don't. Will need a few more defines perhaps to cover
the masks of SELF_TEST, INT_INVERT, FULL_RES, Justify and Range.
>
> #define ADXL345_DEVID 0xE5
>
> struct adxl345_data {
> const struct adxl345_chip_info *info;
> struct regmap *regmap;
> - u8 data_range;
> };
>
> #define ADXL345_CHANNEL(index, axis) { \
> @@ -219,14 +219,13 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
> data = iio_priv(indio_dev);
> data->regmap = regmap;
> /* Enable full-resolution mode */
> - data->data_range = ADXL345_DATA_FORMAT_FULL_RES;
> data->info = device_get_match_data(dev);
> if (!data->info)
> return -ENODEV;
>
> - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT,
> - data->data_range);
> - if (ret < 0)
> + ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT,
> + ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES);
> + if (ret)
> return dev_err_probe(dev, ret, "Failed to set data range\n");
>
> indio_dev->name = data->info->name;
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: iio: health: maxim,max30102: add max30101
From: Jonathan Cameron @ 2024-03-25 20:25 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Javier Carrasco, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matt Ranostay, linux-iio,
devicetree, linux-kernel
In-Reply-To: <b37af0e0-c707-48e0-980d-4866b9a662db@linaro.org>
On Mon, 25 Mar 2024 13:18:14 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> On 24/03/2024 20:20, Javier Carrasco wrote:
> > The Maxim max30101 is the replacement for the max30105, which is no
> > longer recommended for future designs.
> >
> > The max30101 does not require new properties, and it can be described
> > with the existing ones for the max30105, which will be used as a
> > fallback compatible.
> >
> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> > ---
> > .../devicetree/bindings/iio/health/maxim,max30102.yaml | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> Best regards,
> Krzysztof
>
>
Thanks,
Series applied to the togreg branch of iio.git and initially pushed out
as testing to get it some build testing from 0-day.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: qcom: Add support for Motorola Moto G (2013)
From: Stanislav Jakubek @ 2024-03-25 20:25 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-arm-msm, devicetree, linux-arm-kernel, phone-devel,
linux-kernel
In-Reply-To: <ffa93b4a-608a-4cf5-b111-0d1f8520afdd@linaro.org>
On Mon, Mar 25, 2024 at 08:28:27PM +0100, Konrad Dybcio wrote:
> On 24.03.2024 3:04 PM, Stanislav Jakubek wrote:
> > Add a device tree for the Motorola Moto G (2013) smartphone based
> > on the Qualcomm MSM8226 SoC.
> >
> > Initially supported features:
> > - Buttons (Volume Down/Up, Power)
> > - eMMC
> > - Hall Effect Sensor
> > - SimpleFB display
> > - TMP108 temperature sensor
> > - Vibrator
> >
> > Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>
> > ---
>
> [...]
>
> > + hob-ram@f500000 {
> > + reg = <0x0f500000 0x40000>,
> > + <0x0f540000 0x2000>;
> > + no-map;
> > + };
>
> Any reason it's in two parts? Should it be one contiguous region, or
> two separate nodes?
>
> lgtm otherwise
Hi Konrad, I copied this from downstream as-is.
According to the downstream docs [1]:
HOB RAM MMAP Device provides ability for userspace to access the
hand over block memory to read out modem related parameters.
And the two regs are the "DHOB partition" and "SHOB partition".
I suppose this is something Motorola (firmware?) specific (since the
downstream compatible is mmi,hob_ram [2]).
Should I split this into 2 nodes - dhob@f500000 and shob@f540000?
Stanislav
[1] https://github.com/LineageOS/android_kernel_motorola_msm8226/blob/cm-14.1/Documentation/devicetree/bindings/misc/hob_ram.txt
[2] https://github.com/LineageOS/android_kernel_motorola_msm8226/blob/cm-14.1/arch/arm/boot/dts/msm8226-moto-common.dtsi#L258
^ permalink raw reply
* Re: [RFC PATCH 2/2] mfd: rohm-bd71828: Add power off functionality
From: Andreas Kemnade @ 2024-03-25 20:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, mazziesaccount,
devicetree, linux-kernel, imx
In-Reply-To: <b5cc609a-b3c5-4994-bcd5-e25a43d33cff@linaro.org>
On Mon, 25 Mar 2024 13:13:13 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> On 24/03/2024 21:12, Andreas Kemnade wrote:
> > struct regmap_irq_chip_data *irq_data;
> > @@ -542,7 +560,18 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
> > ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
> > NULL, 0, regmap_irq_get_domain(irq_data));
> > if (ret)
> > - dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
> > + return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
> > +
> > + if (of_device_is_system_power_controller(i2c->dev.of_node)) {
> > + if (!pm_power_off) {
> > + bd71828_dev = i2c;
> > + pm_power_off = bd71828_power_off;
> > + ret = devm_add_action_or_reset(&i2c->dev,
> > + bd71828_remove_poweroff,
> > + NULL);
> > + } else
> > + dev_warn(&i2c->dev, "Poweroff callback already assigned\n");
>
> Missing {}
>
> Please run scripts/checkpatch.pl and fix reported warnings. Some
> warnings can be ignored, but the code here looks like it needs a fix.
> Feel free to get in touch if the warning is not clear.
>
No, it does not complain about the {}. I was a bit unsure whether it is
required or not, but I was sure that checkpatch.pl does catch such things.
Yes, documentation clearly says that braces are required in those cases.
Regards,
Andreas
^ permalink raw reply
* Re: [RFC PATCH 2/2] mfd: rohm-bd71828: Add power off functionality
From: Andreas Kemnade @ 2024-03-25 20:14 UTC (permalink / raw)
To: Lee Jones
Cc: Matti Vaittinen, robh+dt, krzysztof.kozlowski+dt, conor+dt,
devicetree, linux-kernel, imx
In-Reply-To: <20240325165134.GP13211@google.com>
On Mon, 25 Mar 2024 16:51:34 +0000
Lee Jones <lee@kernel.org> wrote:
> On Mon, 25 Mar 2024, Lee Jones wrote:
>
> > On Mon, 25 Mar 2024, Matti Vaittinen wrote:
> >
> > > Hi Andreas,
> > >
> > > On 3/25/24 14:16, Andreas Kemnade wrote:
> > > > On Mon, 25 Mar 2024 13:31:15 +0200
> > > > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> > > >
> > > > > On 3/24/24 22:12, Andreas Kemnade wrote:
> > > > > > Since the chip can power off the system, add the corresponding
> > > > > > functionality.
> > > > > > Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2
> > > > > > No information source about the magic numbers found.
> > > > >
> > > > > Oh, interesting repository :) Thanks for linking to it! I didn't know
> > > > > someone had reworked this driver...
> > > > >
> > > > which btw: contains this interesting snippet (output from fdtdump)
> > > > bd71828-i2c@4b {
> > > > reg = <0x0000004b>;
> > > > compatible = "rohm,bd71828";
> > > > gpio_int = <0x00000008 0x00000013 0x00000001>;
> > > > gpio_wdogb = <0x00000039 0x00000018 0x00000001>;
> > > > #address-cells = <0x00000001>;
> > > > #size-cells = <0x00000000>;
> > > > pmic@4b {
> > > > compatible = "rohm,bd71828";
> > > > regulators {
> > > > BUCK1 {
> > > > regulator-name = "buck1";
> > > >
> > > >
> > > > and to make it work since basically no regulators are registered
> > > > instead just some regmap_write()s are done to configure something
> > > > in probe(). It is a pitfall to think that the information below pmic@4b
> > > > is used, especially since it is not that obvious in the source.
> >
> > Odd! Not only did I not receive the original patch, I also did not
> > receive your response Andreas. Spam is empty too.
> >
> > LORE too: https://lore.kernel.org/all/?q=%22mfd:%20rohm-bd71828:%20Add%20power%20off%20functionality%22
>
> Super weird! They just all came through.
>
> The LORE link above is now working too.
>
> I suspect an issue with kernel.org.
>
Well, no, emails from me to everyone sitting behind a mail server without IPv6
address were affected. TCP ACK (3rd part of the handshake) was eaten up by
something. Whatever... So nothing to blame kernel.org besides not having IPv6.
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH v5 4/7] iio: adc: ad7380: add support for pseudo-differential parts
From: David Lechner @ 2024-03-25 20:14 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mark Brown, Julien Stephan, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, linux-iio, devicetree, linux-kernel,
kernel test robot
In-Reply-To: <20240325200625.5a07cec4@jic23-huawei>
On Mon, Mar 25, 2024 at 3:06 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> >
> > But given that this is a common pattern in many IIO drivers, maybe we
> > make a devm_regulator_get_enable_get_voltage()? This would return the
> > voltage on success or an error code. (If the regulator subsystem
> > doesn't want this maybe we could have
> > devm_iio_regulator_get_enable_get_voltage()).
> >
> > If the dev_err_probe() calls were included in
> > devm_regulator_get_enable_get_voltage(), then the 10+ lines of code
> > here and in many other drivers to get the regulator, enable it, add
> > the reset action and get the voltage could be reduced to 3 lines.
>
> I like this proposal a lot. RFC, so it's visible outside the depths
> of this thread?
Yes, I can send an RFC separately so it doesn't hold up this patch/series.
> Particularly good as it will keep the regulator opaque in the same
> fashion as devm_regulator_get_enabled()
>
> As you say, we have a 'lot' of instances of this (quick grep
> suggests > 50 in IIO alone and smaller numbers elsewhere).
>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm64: marvell: add solidrun cn9130 clearfog boards
From: Josua Mayer @ 2024-03-25 20:12 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Yazan Shhady, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <e1836cb6-64cd-4866-9c0a-f0dda096aa18@linaro.org>
Am 25.03.24 um 20:34 schrieb Krzysztof Kozlowski:
> On 22/03/2024 11:08, Josua Mayer wrote:
>> Am 21.03.24 um 22:47 schrieb Josua Mayer:
>>> Add bindings for SolidRun Clearfog boards, using a new SoM based on
>>> CN9130 SoC.
>>> The carrier boards are identical to the older Armada 388 based Clearfog
>>> boards. For consistency the carrier part of compatible strings are
>>> copied, including the established "-a1" suffix.
>>>
>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>> ---
>>> .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>> index 16d2e132d3d1..36bdfd1bedd9 100644
>>> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>> @@ -82,4 +82,16 @@ properties:
>>> - const: marvell,armada-ap807-quad
>>> - const: marvell,armada-ap807
>>>
>>> + - description:
>>> + SolidRun CN9130 clearfog family single-board computers
>>> + items:
>>> + - enum:
>>> + - solidrun,clearfog-base-a1
>>> + - solidrun,clearfog-pro-a1
>>> + - const: solidrun,clearfog-a1
>>> + - const: solidrun,cn9130-sr-som
>>> + - const: marvell,cn9130
>>> + - const: marvell,armada-ap807-quad
>>> + - const: marvell,armada-ap807
>>> +
>>> additionalProperties: true
>> Before merging I would like some feedback about adding
>> another product later, to ensure the compatibles above
>> are adequate? In particular:
>> - sequence of soc, cp, carrier compatibles
>> - name of som compatible
>>
>> Draft for future bindings:
>> - description:
>> SolidRun CN9130 SoM based single-board computers
>> with 1 external CP on the Carrier.
>> items:
>> - enum:
>> - solidrun,cn9131-solidwan
>> - const: marvell,cn9131
>> - const: solidrun,cn9130-sr-som
> This does not look correct. cn9131 is not compatible with your som.
This is partially my question.
I considered changing the som to "cn913x-sr-som".
The SoM itself is always 9130, it contains the base SoC
with 1x AP and 1x CP in a single chip.
9131 and 9132 <happen> on the carrier boards.
>
>> - const: marvell,cn9130
> SoCs are compatible only in some cases, e.g. one is a subset of another
> like stripped out of modem. Are you sure this is your case?
This is more complex, CN9131 and CN9132 are not single SoCs.
A "9132" is instantiated by connecting two southbridge chips
via a Marvell defined bus, each providing additional IO
such as network, i2c, gpio.
Note that even the first, "9130", while a single chip, contains two dies:
An "AP" (Application Processor I assume) with very limited IO (1xsdio, 1xi2c),
and a "CP" (Communication Processor I assume) with lots of IO.
This CP as far as I know today is identical to the southbridges
mentioned above.
>> - const: marvell,armada-ap807-quad
>> - const: marvell,armada-ap807
> Anyway, 6 compatibles is beyond useful amount. What are you expressing
> here?
I copied this part from the examples earlier in the file, such as:
- description: Armada CN9132 SoC with two external CPs
items:
- const: marvell,cn9132
- const: marvell,cn9131
- const: marvell,cn9130
- const: marvell,armada-ap807-quad
- const: marvell,armada-ap807
> Why is this even armada ap807?
We noticed ap807 != ap806 (cn913x != 8040),
because the thermal sensor coefficients converting
raw values to celsius differed.
^ permalink raw reply
* Re: [PATCH v2 3/4] dt-bindings: rng: Add vmgenid support
From: Landge, Sudan @ 2024-03-25 20:11 UTC (permalink / raw)
To: Rob Herring, Sudan Landge
Cc: tytso, Jason, krzysztof.kozlowski+dt, conor+dt,
sathyanarayanan.kuppuswamy, thomas.lendacky, dan.j.williams,
devicetree, linux-kernel, graf, dwmw, bchalios, xmarcalx
In-Reply-To: <20240325150609.GA3477574-robh@kernel.org>
On 25/03/2024 15:06, Rob Herring wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Thu, Mar 21, 2024 at 02:51:04AM +0000, Sudan Landge wrote:
>> 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 chose 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.
>>
>> The point of vmgenid is to provide a mechanism to discover a GUID when
>> the execution state of a virtual machine changes and the simplest
>> way to do it is pass a memory location and an interrupt via devicetree.
>> It would complicate things unnecessarily if instead of using devicetree,
>> we try to implement a new protocol or modify other protocols to somehow
>> provide the same functionility.
>>
>> We believe that adding a devicetree binding for vmgenid is a simpler,
>> better alternative to provide the same functionality and will allow
>> such hypervisors as mentioned above to continue using devicetree.
>>
>> 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>
>> ---
>> .../devicetree/bindings/rng/vmgenid.yaml | 57 +++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 58 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/rng/vmgenid.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/rng/vmgenid.yaml b/Documentation/devicetree/bindings/rng/vmgenid.yaml
>> new file mode 100644
>> index 000000000000..4b6ab62cc2ae
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/rng/vmgenid.yaml
>> @@ -0,0 +1,57 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/rng/vmgenid.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Virtual Machine Generation Counter ID device
>> +
>> +maintainers:
>> + - Jason A. Donenfeld <Jason@zx2c4.com>
>> +
>> +description:
>> + Firmwares or hypervisors can use this devicetree to describe
>> + interrupts and the shared resources to inject a Virtual Machine Generation
>> + counter.
>> +
>> +properties:
>> + compatible:
>> + const: linux,vmgenctr
>
> Why 'linux'? It should be named for a particular host implementation
> (and that implementation's bugs/quirks). However, this thing is simple
> enough we can perhaps avoid that here. As the interface is defined by
> Microsoft, then perhaps they should be the vendor here.
>
We chose "linux" because the current implementation and usage of
devicetree was Linux specific. However, I think "virtual" would be a
better choice than "Microsoft" since this is a generic virtual device
that could be configured by any hypervisor or firmware not owned or
related to Microsoft. I have updated this as part of the new version if
it looks good. I don't have a strong opinion for "virtual" though so if
that is the right choice as per you I can update it.
>> +
>> + "#interrupt-cells":
>> + const: 3
>> + description: |
>> + The 1st cell is the interrupt type.
>> + The 2nd cell contains the interrupt number for the interrupt type.
>> + The 3rd cell is for trigger type and level flags.
>> +
>> + interrupt-controller: true
>
> Why is this device an interrupt controller?
>
My devicetree references I took initially were incorrect which led to
the addition of this, I have removed this in the next version. Sorry
about that.
>> +
>> + reg:
>> + description: |
>> + specifies the base physical address and
>> + size of the regions in memory which holds the VMGenID counter.
>
> Odd wrapping, but drop unless you have something specific to say about
> region like perhaps the layout of the registers. Or maybe thats defined
> somewhere else?
>
> Does the spec say anything about endianness or access size? DT assumes
> native endianness by default. We have properties to deal these, but
> would be better to be explicit if that's defined already.
>
The spec doesn't mention anything about the endianness but, I have
updated the description with some more data.
>> + maxItems: 1
>> +
>> + interrupts:
>> + description: |
>
> Don't need '|' if no formatting.
>
>> + interrupt used to notify that a new VMGenID counter is available.
>> + The interrupt should be Edge triggered.
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + rng@80000000 {
>> + compatible = "linux,vmgenctr";
>> + reg = <0x80000000 0x1000>;
>> + interrupts = <0x00 0x23 0x01>;
>> + };
>> +
>> +...
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 43b39956694a..cf4b2e10fb49 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -18431,6 +18431,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/vmgenid.yaml
>> F: drivers/char/random.c
>> F: drivers/virt/vmgenid.c
>>
>> --
>> 2.40.1
>>
>>
Thank you for the feedback, I have pushed a new version of the patch to
address the review comments.
^ permalink raw reply
* Re: [PATCH v5 4/7] iio: adc: ad7380: add support for pseudo-differential parts
From: Jonathan Cameron @ 2024-03-25 20:06 UTC (permalink / raw)
To: David Lechner, Mark Brown
Cc: Julien Stephan, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, linux-iio, devicetree, linux-kernel,
kernel test robot
In-Reply-To: <CAMknhBGmM7yt1JR1tW4SS5RLGpN9PtnMrf0WvZ-bhU-gSv3YUQ@mail.gmail.com>
> > > + /*
> > > + * pseudo-differential chips have common mode supplies for the negative
> > > + * input pin.
> > > + */
> > > + for (i = 0; i < st->chip_info->num_vcm_supplies; i++) {
> > > + struct regulator *vcm;
> > > +
> > > + vcm = devm_regulator_get_optional(&spi->dev,
> >
> > Why optional?
> >
> > > + st->chip_info->vcm_supplies[i]);
> > > + if (IS_ERR(vcm))
> >
> > This will fail if it's not there, so I'm guessing you are using this to avoid
> > getting to the regulator_get_voltage? If it's not present I'd rely on that
> > failing rather than the confusing handling here.
> >
> > When the read of voltage wasn't in probe this would have resulted in a problem
> > much later than initial setup, now it is, we are just pushing it down a few lines.
> >
> > Arguably we could have a devm_regulator_get_not_dummy()
> > that had same implementation to as get_optional() but whilst it's called that
> > I think it's confusing to use like this.
>
> Despite the misleading naming, I guess I am used to
> devm_regulator_get_optional() by now having used it enough times.
> Since it fails either way though, technically both ways seem fine so I
> can't really argue for one over the other.
>
> But given that this is a common pattern in many IIO drivers, maybe we
> make a devm_regulator_get_enable_get_voltage()? This would return the
> voltage on success or an error code. (If the regulator subsystem
> doesn't want this maybe we could have
> devm_iio_regulator_get_enable_get_voltage()).
>
> If the dev_err_probe() calls were included in
> devm_regulator_get_enable_get_voltage(), then the 10+ lines of code
> here and in many other drivers to get the regulator, enable it, add
> the reset action and get the voltage could be reduced to 3 lines.
I like this proposal a lot. RFC, so it's visible outside the depths
of this thread?
Particularly good as it will keep the regulator opaque in the same
fashion as devm_regulator_get_enabled()
As you say, we have a 'lot' of instances of this (quick grep
suggests > 50 in IIO alone and smaller numbers elsewhere).
Jonathan
>
> >
> > > + return dev_err_probe(&spi->dev, PTR_ERR(vcm),
> > > + "Failed to get %s regulator\n",
> > > + st->chip_info->vcm_supplies[i]);
> > > +
> > > + ret = regulator_enable(vcm);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = devm_add_action_or_reset(&spi->dev,
> > > + ad7380_regulator_disable, vcm);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = regulator_get_voltage(vcm);
> >
> > I'd let this fail if we have a dummy regulator.
> >
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + st->vcm_mv[i] = ret / 1000;
> > > + }
> > > +
>
^ permalink raw reply
* Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
From: Jeff Johnson @ 2024-03-25 20:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Kalle Valo
Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
devicetree, linux-kernel, linux-wireless, linux-arm-msm,
linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <CAMRc=Mc2Tc8oHr5NVo=aHAADkJtGCDAVvJs+7V-19m2zGi-vbw@mail.gmail.com>
On 3/25/2024 7:09 AM, Bartosz Golaszewski wrote:
> On Mon, Mar 25, 2024 at 2:57 PM Kalle Valo <kvalo@kernel.org> wrote:
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> Add a PCI compatible for the ATH11K module on QCA6390 and describe the
>>> power inputs from the PMU that it consumes.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> [...]
>>
>>> +allOf:
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + const: pci17cb,1101
>>> + then:
>>> + required:
>>> + - vddrfacmn-supply
>>> + - vddaon-supply
>>> + - vddwlcx-supply
>>> + - vddwlmx-supply
>>> + - vddrfa0p8-supply
>>> + - vddrfa1p2-supply
>>> + - vddrfa1p7-supply
>>> + - vddpcie0p9-supply
>>> + - vddpcie1p8-supply
>>
>> I don't know DT well enough to know what the "required:" above means,
>> but does this take into account that there are normal "plug&play" type
>> of QCA6390 boards as well which don't need any DT settings?
>>
>
> Do they require a DT node though for some reason?
I would not expect the "PC" flavor of the card to require DT.
The "mobile" and "automotive" flavors would probably require it.
^ permalink raw reply
* Re: [PATCH 3/4] dt-bindings: rtc: digicolor-rtc: convert to dtschema
From: Krzysztof Kozlowski @ 2024-03-25 20:03 UTC (permalink / raw)
To: Javier Carrasco, Rob Herring
Cc: Alexandre Belloni, Krzysztof Kozlowski, Conor Dooley,
Baruch Siach, linux-rtc, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <f765a609-529c-4987-812f-9135041f63d5@gmail.com>
On 25/03/2024 20:46, Javier Carrasco wrote:
> On 3/25/24 17:01, Rob Herring wrote:
>> On Sat, Mar 23, 2024 at 11:46:15PM +0100, Javier Carrasco wrote:
>>> Convert existing binding to dtschema to support validation.
>>>
>>> The binding has been renamed to match its compatible. Apart from that,
>>> it is a direct conversion with no additions.
>>>
>>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>>> ---
>>> .../devicetree/bindings/rtc/cnxt,cx92755-rtc.yaml | 37 ++++++++++++++++++++++
>>> .../devicetree/bindings/rtc/digicolor-rtc.txt | 17 ----------
>>> 2 files changed, 37 insertions(+), 17 deletions(-)
>>
>> This could probably just go into trivial-rtc.yaml.
>>
>> Rob
>
> Does it make no difference if the reg property is a single address or
> address + size? trivial-rtc.yaml does no specify that ('an address' is
> mentioned), and I don't know if it is obvious for someone who wants to
> use this device.
First, you can answer to this by yourself: where do you have the "size"
documented? You will see that nowhere, because your description is
actually redundant and should be dropped. So if "nowhere", then your
binding is the same as trivial-rtc.
The presence of size is defined by the bus, not by this binding.
Therefore you do not have to tell anyone that it is address+size,
because it is obvious from the bus. And from device datasheet (bindings
are not replacement of datasheets).
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 0/3] QCM2290 LMH
From: Krzysztof Kozlowski @ 2024-03-25 19:59 UTC (permalink / raw)
To: Nícolas F. R. A. Prado, Konrad Dybcio
Cc: Bjorn Andersson, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Thara Gopinath, Amit Kucheria, Marijn Suijten, linux-arm-msm,
linux-pm, devicetree, linux-kernel, Dmitry Baryshkov, stable,
Loic Poulain
In-Reply-To: <d8ed4e6c-549f-4c04-b38a-2d788df8b707@notapiano>
On 20/03/2024 20:08, Nícolas F. R. A. Prado wrote:
>> Loic Poulain (1):
>> arm64: dts: qcom: qcm2290: Add LMH node
>>
>> Documentation/devicetree/bindings/thermal/qcom-lmh.yaml | 12 ++++++++----
>> arch/arm64/boot/dts/qcom/qcm2290.dtsi | 14 +++++++++++++-
>> drivers/thermal/qcom/lmh.c | 3 +++
>> 3 files changed, 24 insertions(+), 5 deletions(-)
>
> Hi,
>
> I've started tracking the results of 'make dtbs_check' on linux-next, and I've
> noticed that on today's next, next-20240320, there's a new warning coming from
> this. The reason is that the DT change has landed, but the binding has not,
> since it goes through a separate tree. I thought the binding was supposed to
> always land before the driver and DT that make use of it, but looking through
There is no such rule. Of course new binding should be documented in
earlier or the same kernel release cycle as users get in, but it's not a
requirement.
> the dt-binding documentation pages I couldn't find anything confirming or
> denying that.
>
> I expect this to happen again in the future, which is why I'm reaching out to
> understand better how to deal with this kind of situation.
Deal as what to do? Are you asking in terms of maintenance of some
subsystem or sending some patches? In this particular case here, I don't
think there is anything on your side to deal with.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3 4/4] virt: vmgenid: add support for devicetree bindings
From: Sudan Landge @ 2024-03-25 19:53 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: <20240325195306.13133-1-sudanl@amazon.com>
- 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/vmgenid.yaml
- Use proper FLAGS to compile with and without ACPI and/or devicetree.
Signed-off-by: Sudan Landge <sudanl@amazon.com>
---
drivers/virt/Kconfig | 1 -
drivers/virt/vmgenid.c | 85 +++++++++++++++++++++++++++++++++++++++---
2 files changed, 80 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 d5394c706bd9..a648465bea43 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -2,7 +2,7 @@
/*
* 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.
*/
@@ -21,6 +21,7 @@ enum { VMGENID_SIZE = 16 };
struct vmgenid_state {
u8 *next_id;
u8 this_id[VMGENID_SIZE];
+ unsigned int irq;
};
static void vmgenid_notify(struct device *device)
@@ -37,10 +38,24 @@ static void vmgenid_notify(struct device *device)
kobject_uevent_env(&device->kobj, KOBJ_CHANGE, envp);
}
+#if IS_ENABLED(CONFIG_ACPI)
static void vmgenid_acpi_handler(acpi_handle handle, u32 event, void *dev)
{
+ (void)handle;
+ (void)event;
vmgenid_notify(dev);
}
+#endif
+
+#if IS_ENABLED(CONFIG_OF)
+static irqreturn_t vmgenid_of_irq_handler(int irq, void *dev)
+{
+ (void)irq;
+ vmgenid_notify(dev);
+
+ return IRQ_HANDLED;
+}
+#endif
static int setup_vmgenid_state(struct vmgenid_state *state, u8 *next_id)
{
@@ -55,6 +70,7 @@ static int setup_vmgenid_state(struct vmgenid_state *state, u8 *next_id)
static int vmgenid_add_acpi(struct device *dev, struct vmgenid_state *state)
{
+#if IS_ENABLED(CONFIG_ACPI)
struct acpi_device *device = ACPI_COMPANION(dev);
struct acpi_buffer parsed = { ACPI_ALLOCATE_BUFFER };
union acpi_object *obj;
@@ -96,6 +112,49 @@ static int vmgenid_add_acpi(struct device *dev, struct vmgenid_state *state)
out:
ACPI_FREE(parsed.pointer);
return ret;
+#else
+ (void)dev;
+ (void)state;
+ return -EINVAL;
+#endif
+}
+
+static int vmgenid_add_of(struct platform_device *pdev, struct vmgenid_state *state)
+{
+#if IS_ENABLED(CONFIG_OF)
+ void __iomem *remapped_ptr;
+ int ret = 0;
+
+ remapped_ptr = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+ if (IS_ERR(remapped_ptr)) {
+ ret = PTR_ERR(remapped_ptr);
+ goto out;
+ }
+
+ ret = setup_vmgenid_state(state, remapped_ptr);
+ if (ret)
+ goto out;
+
+ state->irq = platform_get_irq(pdev, 0);
+ if (state->irq < 0) {
+ ret = state->irq;
+ goto out;
+ }
+ 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;
+
+out:
+ return ret;
+#else
+ (void)dev;
+ (void)state;
+ return -EINVAL;
+#endif
}
static int vmgenid_add(struct platform_device *pdev)
@@ -108,7 +167,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);
@@ -116,18 +178,31 @@ static int vmgenid_add(struct platform_device *pdev)
return ret;
}
-static const struct acpi_device_id vmgenid_ids[] = {
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vmgenid_of_ids[] = {
+ { .compatible = "linux,vmgenctr", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, vmgenid_of_ids);
+#endif
+
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id vmgenid_acpi_ids[] = {
{ "VMGENCTR", 0 },
{ "VM_GEN_COUNTER", 0 },
{ }
};
-MODULE_DEVICE_TABLE(acpi, vmgenid_ids);
+MODULE_DEVICE_TABLE(acpi, vmgenid_acpi_ids);
+#endif
static struct platform_driver vmgenid_plaform_driver = {
.probe = vmgenid_add,
.driver = {
.name = "vmgenid",
- .acpi_match_table = ACPI_PTR(vmgenid_ids),
+ .acpi_match_table = ACPI_PTR(vmgenid_acpi_ids),
+#if IS_ENABLED(CONFIG_OF)
+ .of_match_table = vmgenid_of_ids,
+#endif
.owner = THIS_MODULE,
},
};
--
2.40.1
^ permalink raw reply related
* Re: [RESEND PATCH v2 4/4] iio: adc: rcar-gyroadc: use for_each_available_child_node_scoped()
From: Jonathan Cameron @ 2024-03-25 19:53 UTC (permalink / raw)
To: devicetree, linux-iio, Rob Herring, Frank Rowand, linux-kernel,
Julia Lawall
Cc: Peter Zijlstra, Andy Shevchenko, Greg Kroah-Hartman, marek.vasut,
Jonathan Cameron
In-Reply-To: <20240225142714.286440-5-jic23@kernel.org>
On Sun, 25 Feb 2024 14:27:14 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Using automated cleanup to replace of_node_put() handling allows for
> a simplfied flow by enabling direct returns on errors.
>
> Non available child nodes should never have been considered; that
> is ones where status != okay and was defined.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Applied to the togreg branch of iio.git and pushed out as testing
for all the normal reasons.
Thanks,
Jonathan
> ---
> drivers/iio/adc/rcar-gyroadc.c | 21 ++++++---------------
> 1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/adc/rcar-gyroadc.c b/drivers/iio/adc/rcar-gyroadc.c
> index d524f2e8e927..15a21d2860e7 100644
> --- a/drivers/iio/adc/rcar-gyroadc.c
> +++ b/drivers/iio/adc/rcar-gyroadc.c
> @@ -318,7 +318,6 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> struct rcar_gyroadc *priv = iio_priv(indio_dev);
> struct device *dev = priv->dev;
> struct device_node *np = dev->of_node;
> - struct device_node *child;
> struct regulator *vref;
> unsigned int reg;
> unsigned int adcmode = -1, childmode;
> @@ -326,7 +325,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> unsigned int num_channels;
> int ret, first = 1;
>
> - for_each_child_of_node(np, child) {
> + for_each_available_child_of_node_scoped(np, child) {
> of_id = of_match_node(rcar_gyroadc_child_match, child);
> if (!of_id) {
> dev_err(dev, "Ignoring unsupported ADC \"%pOFn\".",
> @@ -352,7 +351,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> num_channels = ARRAY_SIZE(rcar_gyroadc_iio_channels_3);
> break;
> default:
> - goto err_e_inval;
> + return -EINVAL;
> }
>
> /*
> @@ -369,7 +368,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> dev_err(dev,
> "Failed to get child reg property of ADC \"%pOFn\".\n",
> child);
> - goto err_of_node_put;
> + return ret;
> }
>
> /* Channel number is too high. */
> @@ -377,7 +376,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> dev_err(dev,
> "Only %i channels supported with %pOFn, but reg = <%i>.\n",
> num_channels, child, reg);
> - goto err_e_inval;
> + return -EINVAL;
> }
> }
>
> @@ -386,7 +385,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> dev_err(dev,
> "Channel %i uses different ADC mode than the rest.\n",
> reg);
> - goto err_e_inval;
> + return -EINVAL;
> }
>
> /* Channel is valid, grab the regulator. */
> @@ -396,8 +395,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> if (IS_ERR(vref)) {
> dev_dbg(dev, "Channel %i 'vref' supply not connected.\n",
> reg);
> - ret = PTR_ERR(vref);
> - goto err_of_node_put;
> + return PTR_ERR(vref);
> }
>
> priv->vref[reg] = vref;
> @@ -422,7 +420,6 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> * we can stop parsing here.
> */
> if (childmode == RCAR_GYROADC_MODE_SELECT_1_MB88101A) {
> - of_node_put(child);
> break;
> }
> }
> @@ -433,12 +430,6 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
> }
>
> return 0;
> -
> -err_e_inval:
> - ret = -EINVAL;
> -err_of_node_put:
> - of_node_put(child);
> - return ret;
> }
>
> static void rcar_gyroadc_deinit_supplies(struct iio_dev *indio_dev)
^ permalink raw reply
* [PATCH v3 3/4] dt-bindings: rng: Add vmgenid support
From: Sudan Landge @ 2024-03-25 19:53 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: <20240325195306.13133-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 chose 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.
The point of vmgenid is to provide a mechanism to discover a GUID when
the execution state of a virtual machine changes and the simplest
way to do it is pass a memory location and an interrupt via devicetree.
It would complicate things unnecessarily if instead of using devicetree,
we try to implement a new protocol or modify other protocols to somehow
provide the same functionility.
We believe that adding a devicetree binding for vmgenid is a simpler,
better alternative to provide the same functionality and will allow
such hypervisors as mentioned above to continue using devicetree.
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>
---
.../devicetree/bindings/rng/vmgenid.yaml | 58 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/vmgenid.yaml
diff --git a/Documentation/devicetree/bindings/rng/vmgenid.yaml b/Documentation/devicetree/bindings/rng/vmgenid.yaml
new file mode 100644
index 000000000000..24643080d6b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/vmgenid.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rng/vmgenid.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtual Machine Generation Counter ID device
+
+maintainers:
+ - Jason A. Donenfeld <Jason@zx2c4.com>
+
+description:
+ Firmwares or hypervisors can use this devicetree to describe
+ interrupts and the shared resources to inject a Virtual Machine Generation
+ counter.
+
+properties:
+ compatible:
+ const: virtual,vmgenctr
+
+ "#interrupt-cells":
+ const: 3
+ description:
+ The 1st cell is the interrupt type.
+ The 2nd cell contains the interrupt number for the interrupt type.
+ The 3rd cell is for trigger type and level flags.
+
+ interrupt-map: true
+
+ reg:
+ description:
+ The 1st cell specifies the base physical address of the 8-byte aligned
+ buffer in guest memory space which is guaranteed not to be used by the
+ operating system.
+ The 2nd cell specifies the size of the buffer which holds the VMGenID.
+ maxItems: 1
+
+ interrupts:
+ description:
+ interrupt used to notify that a new VMGenID counter is available.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ rng@80000000 {
+ compatible = "virtual,vmgenctr";
+ reg = <0x80000000 0x1000>;
+ interrupts = <0x00 0x23 0x01>;
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index de6a64b248ae..e295d2f50af4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18461,6 +18461,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/vmgenid.yaml
F: drivers/char/random.c
F: drivers/virt/vmgenid.c
--
2.40.1
^ permalink raw reply related
* [PATCH v3 2/4] virt: vmgenid: change implementation to use a platform driver
From: Sudan Landge @ 2024-03-25 19:53 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: <20240325195306.13133-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 | 101 ++++++++++++++++++++++++++++++-----------
1 file changed, 74 insertions(+), 27 deletions(-)
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index ea956df02874..d5394c706bd9 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -11,6 +11,8 @@
#include <linux/module.h>
#include <linux/acpi.h>
#include <linux/random.h>
+#include <acpi/actypes.h>
+#include <linux/platform_device.h>
ACPI_MODULE_NAME("vmgenid");
@@ -21,9 +23,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,22 +34,34 @@ 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 int vmgenid_add(struct acpi_device *device)
+static void vmgenid_acpi_handler(acpi_handle handle, u32 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_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;
-
status = acpi_evaluate_object(device->handle, "ADDR", NULL, &parsed);
if (ACPI_FAILURE(status)) {
ACPI_EXCEPTION((AE_INFO, status, "Evaluating ADDR"));
@@ -63,19 +77,42 @@ 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);
+
+ ret = setup_vmgenid_state(state,
+ (u8 *)devm_memremap(&device->dev, phys_addr,
+ VMGENID_SIZE, MEMREMAP_WB));
+ 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;
}
@@ -84,20 +121,30 @@ static const struct acpi_device_id vmgenid_ids[] = {
{ "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
- }
+static struct platform_driver vmgenid_plaform_driver = {
+ .probe = vmgenid_add,
+ .driver = {
+ .name = "vmgenid",
+ .acpi_match_table = ACPI_PTR(vmgenid_ids),
+ .owner = THIS_MODULE,
+ },
};
-module_acpi_driver(vmgenid_driver);
+static int vmgenid_platform_device_init(void)
+{
+ return platform_driver_register(&vmgenid_plaform_driver);
+}
+
+static void vmgenid_platform_device_exit(void)
+{
+ platform_driver_unregister(&vmgenid_plaform_driver);
+}
+
+module_init(vmgenid_platform_device_init)
+module_exit(vmgenid_platform_device_exit)
-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.40.1
^ permalink raw reply related
* [PATCH v3 1/4] virt: vmgenid: rearrange code to make review easier
From: Sudan Landge @ 2024-03-25 19:53 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: <20240325195306.13133-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 | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index b67a28da4702..ea956df02874 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,20 +79,6 @@ 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 },
--
2.40.1
^ permalink raw reply related
* [PATCH v3 0/4] virt: vmgenid: Add devicetree bindings support
From: Sudan Landge @ 2024-03-25 19:53 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 chose 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.
The point of vmgenid is to provide a mechanism to discover a GUID when
the execution state of a virtual machine changes and the simplest
way to do it is pass a memory location and an interrupt via devicetree.
It would complicate things unnecessarily if instead of using devicetree,
we try to implement a new protocol or modify other protocols to somehow
provide the same functionility.
We believe that adding a devicetree binding for vmgenid is a simpler,
better alternative to provide the same functionality and will allow
such hypervisors as mentioned above to continue using devicetree.
Addtional notes:
While adding the devicetree support we considered re-using existing
structures/code to avoid duplication 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/vmgenid/vmgenid.yaml
```
and the patches were verified with:
`scripts/checkpatch.pl --strict v1-000*`.
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 (4):
virt: vmgenid: rearrange code to make review easier
virt: vmgenid: change implementation to use a platform driver
dt-bindings: rng: Add vmgenid support
virt: vmgenid: add support for devicetree bindings
.../devicetree/bindings/rng/vmgenid.yaml | 58 ++++++
MAINTAINERS | 1 +
drivers/virt/Kconfig | 1 -
drivers/virt/vmgenid.c | 194 ++++++++++++++----
4 files changed, 217 insertions(+), 37 deletions(-)
create mode 100644 Documentation/devicetree/bindings/rng/vmgenid.yaml
base-commit: 8e938e39866920ddc266898e6ae1fffc5c8f51aa
--
2.40.1
^ permalink raw reply
* Re: [PATCH v2 0/3] QCM2290 LMH
From: Konrad Dybcio @ 2024-03-25 19:48 UTC (permalink / raw)
To: Nícolas F. R. A. Prado
Cc: Bjorn Andersson, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Thara Gopinath, Amit Kucheria, Marijn Suijten, linux-arm-msm,
linux-pm, devicetree, linux-kernel, Dmitry Baryshkov, stable,
Loic Poulain
In-Reply-To: <d8ed4e6c-549f-4c04-b38a-2d788df8b707@notapiano>
On 20.03.2024 8:08 PM, Nícolas F. R. A. Prado wrote:
> On Sat, Mar 09, 2024 at 02:15:01PM +0100, Konrad Dybcio wrote:
>> Wire up LMH on QCM2290 and fix a bad bug while at it.
>>
>> P1-2 for thermal, P3 for qcom
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> Changes in v2:
>> - Pick up tags
>> - Fix a couple typos in commit messages
>> - Drop stray msm8998 binding addition
>> - Link to v1: https://lore.kernel.org/r/20240308-topic-rb1_lmh-v1-0-50c60ffe1130@linaro.org
>>
>> ---
>> Konrad Dybcio (2):
>> dt-bindings: thermal: lmh: Add QCM2290 compatible
>> thermal: qcom: lmh: Check for SCM availability at probe
>>
>> Loic Poulain (1):
>> arm64: dts: qcom: qcm2290: Add LMH node
>>
>> Documentation/devicetree/bindings/thermal/qcom-lmh.yaml | 12 ++++++++----
>> arch/arm64/boot/dts/qcom/qcm2290.dtsi | 14 +++++++++++++-
>> drivers/thermal/qcom/lmh.c | 3 +++
>> 3 files changed, 24 insertions(+), 5 deletions(-)
>
> Hi,
>
> I've started tracking the results of 'make dtbs_check' on linux-next, and I've
> noticed that on today's next, next-20240320, there's a new warning coming from
> this. The reason is that the DT change has landed, but the binding has not,
> since it goes through a separate tree. I thought the binding was supposed to
> always land before the driver and DT that make use of it, but looking through
> the dt-binding documentation pages I couldn't find anything confirming or
> denying that.
Yes, that's the ideal way of things happening..
>
> I expect this to happen again in the future, which is why I'm reaching out to
> understand better how to deal with this kind of situation.
..but due to the kernel dev process, doing that across multiple trees would
either require constant agreements on immutable branches containing bindings,
mixing patches across trees, or delaying dts changes by a cycle or so
Konrad
^ permalink raw reply
* Re: [PATCH 3/4] dt-bindings: rtc: digicolor-rtc: convert to dtschema
From: Javier Carrasco @ 2024-03-25 19:46 UTC (permalink / raw)
To: Rob Herring
Cc: Alexandre Belloni, Krzysztof Kozlowski, Conor Dooley,
Baruch Siach, linux-rtc, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20240325160152.GA4035876-robh@kernel.org>
On 3/25/24 17:01, Rob Herring wrote:
> On Sat, Mar 23, 2024 at 11:46:15PM +0100, Javier Carrasco wrote:
>> Convert existing binding to dtschema to support validation.
>>
>> The binding has been renamed to match its compatible. Apart from that,
>> it is a direct conversion with no additions.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>> ---
>> .../devicetree/bindings/rtc/cnxt,cx92755-rtc.yaml | 37 ++++++++++++++++++++++
>> .../devicetree/bindings/rtc/digicolor-rtc.txt | 17 ----------
>> 2 files changed, 37 insertions(+), 17 deletions(-)
>
> This could probably just go into trivial-rtc.yaml.
>
> Rob
Does it make no difference if the reg property is a single address or
address + size? trivial-rtc.yaml does no specify that ('an address' is
mentioned), and I don't know if it is obvious for someone who wants to
use this device.
Best regards,
Javier Carrasco
^ permalink raw reply
* Re: [PATCH v16 7/9] usb: dwc3: qcom: Refactor IRQ handling in glue driver
From: Johan Hovold @ 2024-03-25 19:45 UTC (permalink / raw)
To: Krishna Kurapati PSSNV
Cc: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, devicetree, linux-arm-msm, linux-usb, linux-kernel,
quic_ppratap, quic_jackp
In-Reply-To: <807015d4-c5ed-4e04-9948-fd1ff894a04e@quicinc.com>
On Mon, Mar 25, 2024 at 10:59:49PM +0530, Krishna Kurapati PSSNV wrote:
> On 3/25/2024 6:53 PM, Johan Hovold wrote:
> > ret = dwc3_qcom_find_num_ports(pdev);
> > if (ret < 0)
> > return ret;
> >
> > qcom->num_ports = ret;
> >
> > It looks like dwc3_qcom_find_num_ports() can also return 0 (e.g. on
> > malformed DT), which also needs to be handled somehow. I missed that
> > earlier.
>
> From what I remember, Konrad mentioned that we might not need to
> support incomplete or improper DT [1].
You still need to make sure that the driver doesn't misbehave on
malformed input.
> Also since this is close to getting merged, can we take up any changes
> for Malformed DT handling later given that only one or two devices are
> present and less likely to be given a malformed DT.
No, and I'm a bit disappointed that you're still trying to take short
cuts to getting these patches merged after I've told you repeatedly that
that is not how upstream works.
Fortunately, this is easily fixed and would both simplify the code
further and reduce the risk of breaking backwards compatibility.
Just change the logic in dwc3_qcom_find_num_ports() so that it returns 1
if "dp_hs_phy_1" is missing, and otherwise you determine the number of
ports by iterating from 2 to DWC3_MAX_PORTS - 1.
Johan
^ permalink raw reply
* Re: [PATCH v2 3/4] clk: qcom: add IPQ9574 interconnect clocks support
From: Konrad Dybcio @ 2024-03-25 19:42 UTC (permalink / raw)
To: Varadarajan Narayanan, andersson, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, quic_anusha,
linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-pm
In-Reply-To: <20240325102036.95484-4-quic_varada@quicinc.com>
On 25.03.2024 11:20 AM, Varadarajan Narayanan wrote:
> Unlike MSM platforms that manage NoC related clocks and scaling
> from RPM, IPQ SoCs dont involve RPM in managing NoC related
> clocks and there is no NoC scaling.
>
> However, there is a requirement to enable some NoC interface
> clocks for accessing the peripheral controllers present on
> these NoCs. Though exposing these as normal clocks would work,
> having a minimalistic interconnect driver to handle these clocks
> would make it consistent with other Qualcomm platforms resulting
> in common code paths. This is similar to msm8996-cbf's usage of
> icc-clk framework.
>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
[...]
>
> +
> +static struct icc_clk_data *icc_ipq9574;
> +
What does this help achieve?
[...]
> +static int noc_clks[] = {
We could probably use indexed identifiers here to avoid confusion:
[ICC_BINDING_NAME] = CLK_BINDING_NAME
> static int gcc_ipq9574_probe(struct platform_device *pdev)
> {
> - return qcom_cc_probe(pdev, &gcc_ipq9574_desc);
> + int ret = qcom_cc_probe(pdev, &gcc_ipq9574_desc);
> + struct icc_provider *provider;
> + struct icc_clk_data *icd;
> + int i;
> +
> + if (ret)
I'd personally prefer if you left ret uninitialized and assigned it
above the if-statement.
> + return dev_err_probe(&pdev->dev, ret, "%s failed\n", __func__);
Please avoid the use of __func__ throughout your change and write
a more useful error message.
> +
> + icd = devm_kmalloc(&pdev->dev, ARRAY_SIZE(noc_clks) * sizeof(*icd),
> + GFP_KERNEL);
devm_kcalloc
> +
> + if (IS_ERR_OR_NULL(icd))
> + return dev_err_probe(&pdev->dev, PTR_ERR(icd),
> + "%s malloc failed\n", __func__);
ditto
> +
> + icc_ipq9574 = icd;
> +
> + for (i = 0; i < ARRAY_SIZE(noc_clks); i++, icd++) {
> + icd->clk = gcc_ipq9574_clks[noc_clks[i]]->hw.clk;
> + if (IS_ERR_OR_NULL(icd->clk)) {
> + dev_err(&pdev->dev, "%s: %d clock not found\n",
> + __func__, noc_clks[i]);
> + return -ENOENT;
return dev_err_probe
> + }
> + icd->name = clk_hw_get_name(&gcc_ipq9574_clks[noc_clks[i]]->hw);
> + }
> +
> + provider = icc_clk_register(&pdev->dev, IPQ_APPS_ID,
> + ARRAY_SIZE(noc_clks), icc_ipq9574);
> + if (IS_ERR_OR_NULL(provider))
> + return dev_err_probe(&pdev->dev, PTR_ERR(provider),
> + "%s: icc_clk_register failed\n", __func__);
ditto
On a second thought, since I'm assuming you're going to expand this to other
IPQ SoCs, it might be useful to factor this out into drivers/clk/qcom/common.c
Konrad
^ permalink raw reply
* [PATCH v9 7/7] clk: rockchip: rk3588: drop RK3588_LINKED_CLK
From: Sebastian Reichel @ 2024-03-25 19:33 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, linux-clk
Cc: Elaine Zhang, Kever Yang, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, huangtao, andy.yan,
Michal Tomek, Ilya K, Chad LeClair, devicetree, linux-rockchip,
Sebastian Reichel, kernel
In-Reply-To: <20240325193609.237182-1-sebastian.reichel@collabora.com>
With the proper GATE_LINK support, we no longer need to keep the
linked clocks always on. Thus it's time to drop the CLK_IS_CRITICAL
flag for them.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/clk/rockchip/clk-rk3588.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
index 1bf84ac44e85..42579b6f74b4 100644
--- a/drivers/clk/rockchip/clk-rk3588.c
+++ b/drivers/clk/rockchip/clk-rk3588.c
@@ -12,9 +12,6 @@
#include <dt-bindings/clock/rockchip,rk3588-cru.h>
#include "clk.h"
-#define RK3588_LINKED_CLK CLK_IS_CRITICAL
-
-
#define RK3588_GRF_SOC_STATUS0 0x600
#define RK3588_PHYREF_ALT_GATE 0xc38
@@ -1439,7 +1436,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
COMPOSITE_NODIV(HCLK_NVM_ROOT, "hclk_nvm_root", mux_200m_100m_50m_24m_p, 0,
RK3588_CLKSEL_CON(77), 0, 2, MFLAGS,
RK3588_CLKGATE_CON(31), 0, GFLAGS),
- COMPOSITE(ACLK_NVM_ROOT, "aclk_nvm_root", gpll_cpll_p, RK3588_LINKED_CLK,
+ COMPOSITE(ACLK_NVM_ROOT, "aclk_nvm_root", gpll_cpll_p, 0,
RK3588_CLKSEL_CON(77), 7, 1, MFLAGS, 2, 5, DFLAGS,
RK3588_CLKGATE_CON(31), 1, GFLAGS),
GATE(ACLK_EMMC, "aclk_emmc", "aclk_nvm_root", 0,
@@ -1668,13 +1665,13 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
RK3588_CLKGATE_CON(42), 9, GFLAGS),
/* vdpu */
- COMPOSITE(ACLK_VDPU_ROOT, "aclk_vdpu_root", gpll_cpll_aupll_p, RK3588_LINKED_CLK,
+ COMPOSITE(ACLK_VDPU_ROOT, "aclk_vdpu_root", gpll_cpll_aupll_p, 0,
RK3588_CLKSEL_CON(98), 5, 2, MFLAGS, 0, 5, DFLAGS,
RK3588_CLKGATE_CON(44), 0, GFLAGS),
COMPOSITE_NODIV(ACLK_VDPU_LOW_ROOT, "aclk_vdpu_low_root", mux_400m_200m_100m_24m_p, 0,
RK3588_CLKSEL_CON(98), 7, 2, MFLAGS,
RK3588_CLKGATE_CON(44), 1, GFLAGS),
- COMPOSITE_NODIV(HCLK_VDPU_ROOT, "hclk_vdpu_root", mux_200m_100m_50m_24m_p, RK3588_LINKED_CLK,
+ COMPOSITE_NODIV(HCLK_VDPU_ROOT, "hclk_vdpu_root", mux_200m_100m_50m_24m_p, 0,
RK3588_CLKSEL_CON(98), 9, 2, MFLAGS,
RK3588_CLKGATE_CON(44), 2, GFLAGS),
COMPOSITE(ACLK_JPEG_DECODER_ROOT, "aclk_jpeg_decoder_root", gpll_cpll_aupll_spll_p, 0,
@@ -1725,9 +1722,9 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
COMPOSITE(ACLK_RKVENC0_ROOT, "aclk_rkvenc0_root", gpll_cpll_npll_p, 0,
RK3588_CLKSEL_CON(102), 7, 2, MFLAGS, 2, 5, DFLAGS,
RK3588_CLKGATE_CON(47), 1, GFLAGS),
- GATE(HCLK_RKVENC0, "hclk_rkvenc0", "hclk_rkvenc0_root", RK3588_LINKED_CLK,
+ GATE(HCLK_RKVENC0, "hclk_rkvenc0", "hclk_rkvenc0_root", 0,
RK3588_CLKGATE_CON(47), 4, GFLAGS),
- GATE(ACLK_RKVENC0, "aclk_rkvenc0", "aclk_rkvenc0_root", RK3588_LINKED_CLK,
+ GATE(ACLK_RKVENC0, "aclk_rkvenc0", "aclk_rkvenc0_root", 0,
RK3588_CLKGATE_CON(47), 5, GFLAGS),
COMPOSITE(CLK_RKVENC0_CORE, "clk_rkvenc0_core", gpll_cpll_aupll_npll_p, 0,
RK3588_CLKSEL_CON(102), 14, 2, MFLAGS, 9, 5, DFLAGS,
@@ -1737,10 +1734,10 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
RK3588_CLKGATE_CON(48), 6, GFLAGS),
/* vi */
- COMPOSITE(ACLK_VI_ROOT, "aclk_vi_root", gpll_cpll_npll_aupll_spll_p, RK3588_LINKED_CLK,
+ COMPOSITE(ACLK_VI_ROOT, "aclk_vi_root", gpll_cpll_npll_aupll_spll_p, 0,
RK3588_CLKSEL_CON(106), 5, 3, MFLAGS, 0, 5, DFLAGS,
RK3588_CLKGATE_CON(49), 0, GFLAGS),
- COMPOSITE_NODIV(HCLK_VI_ROOT, "hclk_vi_root", mux_200m_100m_50m_24m_p, RK3588_LINKED_CLK,
+ COMPOSITE_NODIV(HCLK_VI_ROOT, "hclk_vi_root", mux_200m_100m_50m_24m_p, 0,
RK3588_CLKSEL_CON(106), 8, 2, MFLAGS,
RK3588_CLKGATE_CON(49), 1, GFLAGS),
COMPOSITE_NODIV(PCLK_VI_ROOT, "pclk_vi_root", mux_100m_50m_24m_p, 0,
@@ -1910,10 +1907,10 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
COMPOSITE(ACLK_VOP_ROOT, "aclk_vop_root", gpll_cpll_dmyaupll_npll_spll_p, 0,
RK3588_CLKSEL_CON(110), 5, 3, MFLAGS, 0, 5, DFLAGS,
RK3588_CLKGATE_CON(52), 0, GFLAGS),
- COMPOSITE_NODIV(ACLK_VOP_LOW_ROOT, "aclk_vop_low_root", mux_400m_200m_100m_24m_p, RK3588_LINKED_CLK,
+ COMPOSITE_NODIV(ACLK_VOP_LOW_ROOT, "aclk_vop_low_root", mux_400m_200m_100m_24m_p, 0,
RK3588_CLKSEL_CON(110), 8, 2, MFLAGS,
RK3588_CLKGATE_CON(52), 1, GFLAGS),
- COMPOSITE_NODIV(HCLK_VOP_ROOT, "hclk_vop_root", mux_200m_100m_50m_24m_p, RK3588_LINKED_CLK,
+ COMPOSITE_NODIV(HCLK_VOP_ROOT, "hclk_vop_root", mux_200m_100m_50m_24m_p, 0,
RK3588_CLKSEL_CON(110), 10, 2, MFLAGS,
RK3588_CLKGATE_CON(52), 2, GFLAGS),
COMPOSITE_NODIV(PCLK_VOP_ROOT, "pclk_vop_root", mux_100m_50m_24m_p, 0,
@@ -2416,7 +2413,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
static struct rockchip_clk_branch rk3588_clk_branches[] = {
GATE_LINK(ACLK_ISP1_PRE, "aclk_isp1_pre", "aclk_isp1_root", ACLK_VI_ROOT, 0, RK3588_CLKGATE_CON(26), 6, GFLAGS),
GATE_LINK(HCLK_ISP1_PRE, "hclk_isp1_pre", "hclk_isp1_root", HCLK_VI_ROOT, 0, RK3588_CLKGATE_CON(26), 8, GFLAGS),
- GATE_LINK(HCLK_NVM, "hclk_nvm", "hclk_nvm_root", ACLK_NVM_ROOT, RK3588_LINKED_CLK, RK3588_CLKGATE_CON(31), 2, GFLAGS),
+ GATE_LINK(HCLK_NVM, "hclk_nvm", "hclk_nvm_root", ACLK_NVM_ROOT, 0, RK3588_CLKGATE_CON(31), 2, GFLAGS),
GATE_LINK(ACLK_USB, "aclk_usb", "aclk_usb_root", ACLK_VO1USB_TOP_ROOT, 0, RK3588_CLKGATE_CON(42), 2, GFLAGS),
GATE_LINK(HCLK_USB, "hclk_usb", "hclk_usb_root", HCLK_VO1USB_TOP_ROOT, 0, RK3588_CLKGATE_CON(42), 3, GFLAGS),
GATE_LINK(ACLK_JPEG_DECODER_PRE, "aclk_jpeg_decoder_pre", "aclk_jpeg_decoder_root", ACLK_VDPU_ROOT, 0, RK3588_CLKGATE_CON(44), 7, GFLAGS),
@@ -2428,9 +2425,9 @@ static struct rockchip_clk_branch rk3588_clk_branches[] = {
GATE_LINK(HCLK_RKVDEC1_PRE, "hclk_rkvdec1_pre", "hclk_rkvdec1_root", HCLK_VDPU_ROOT, 0, RK3588_CLKGATE_CON(41), 4, GFLAGS),
GATE_LINK(ACLK_RKVDEC1_PRE, "aclk_rkvdec1_pre", "aclk_rkvdec1_root", ACLK_VDPU_ROOT, 0, RK3588_CLKGATE_CON(41), 5, GFLAGS),
GATE_LINK(ACLK_HDCP0_PRE, "aclk_hdcp0_pre", "aclk_vo0_root", ACLK_VOP_LOW_ROOT, 0, RK3588_CLKGATE_CON(55), 9, GFLAGS),
- GATE_LINK(HCLK_VO0, "hclk_vo0", "hclk_vo0_root", HCLK_VOP_ROOT, RK3588_LINKED_CLK, RK3588_CLKGATE_CON(55), 5, GFLAGS),
+ GATE_LINK(HCLK_VO0, "hclk_vo0", "hclk_vo0_root", HCLK_VOP_ROOT, 0, RK3588_CLKGATE_CON(55), 5, GFLAGS),
GATE_LINK(ACLK_HDCP1_PRE, "aclk_hdcp1_pre", "aclk_hdcp1_root", ACLK_VO1USB_TOP_ROOT, 0, RK3588_CLKGATE_CON(59), 6, GFLAGS),
- GATE_LINK(HCLK_VO1, "hclk_vo1", "hclk_vo1_root", HCLK_VO1USB_TOP_ROOT, RK3588_LINKED_CLK, RK3588_CLKGATE_CON(59), 9, GFLAGS),
+ GATE_LINK(HCLK_VO1, "hclk_vo1", "hclk_vo1_root", HCLK_VO1USB_TOP_ROOT, 0, RK3588_CLKGATE_CON(59), 9, GFLAGS),
GATE_LINK(ACLK_AV1_PRE, "aclk_av1_pre", "aclk_av1_root", ACLK_VDPU_ROOT, 0, RK3588_CLKGATE_CON(68), 1, GFLAGS),
GATE_LINK(PCLK_AV1_PRE, "pclk_av1_pre", "pclk_av1_root", HCLK_VDPU_ROOT, 0, RK3588_CLKGATE_CON(68), 4, GFLAGS),
GATE_LINK(HCLK_SDIO_PRE, "hclk_sdio_pre", "hclk_sdio_root", HCLK_NVM, 0, RK3588_CLKGATE_CON(75), 1, GFLAGS),
--
2.43.0
^ permalink raw reply related
* [PATCH v9 6/7] clk: rockchip: implement linked gate clock support
From: Sebastian Reichel @ 2024-03-25 19:33 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, linux-clk
Cc: Elaine Zhang, Kever Yang, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, huangtao, andy.yan,
Michal Tomek, Ilya K, Chad LeClair, devicetree, linux-rockchip,
Sebastian Reichel, kernel
In-Reply-To: <20240325193609.237182-1-sebastian.reichel@collabora.com>
Recent Rockchip SoCs have a new hardware block called Native Interface
Unit (NIU), which gates clocks to devices behind them. These clock
gates will only have a running output clock when all of the following
conditions are met:
1. the parent clock is enabled
2. the enable bit is set correctly
3. the linked clock is enabled
To handle them this code registers them as a normal gate type clock,
which takes care of condition 1 + 2. The linked clock is handled by
using runtime PM clocks. Handling it via runtime PM requires setting
up a struct device for each of these clocks with a driver attached
to use the correct runtime PM operations. Thus the complete handling
of these clocks has been moved into its own driver.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/clk/rockchip/Makefile | 1 +
drivers/clk/rockchip/clk-rk3588.c | 23 +------
drivers/clk/rockchip/clk.c | 52 ++++++++++++++++
drivers/clk/rockchip/clk.h | 25 ++++++++
drivers/clk/rockchip/gate-link.c | 99 +++++++++++++++++++++++++++++++
5 files changed, 179 insertions(+), 21 deletions(-)
create mode 100644 drivers/clk/rockchip/gate-link.c
diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile
index 36894f6a7022..179be95c6ffb 100644
--- a/drivers/clk/rockchip/Makefile
+++ b/drivers/clk/rockchip/Makefile
@@ -13,6 +13,7 @@ clk-rockchip-y += clk-inverter.o
clk-rockchip-y += clk-mmc-phase.o
clk-rockchip-y += clk-muxgrf.o
clk-rockchip-y += clk-ddr.o
+clk-rockchip-y += gate-link.o
clk-rockchip-$(CONFIG_RESET_CONTROLLER) += softrst.o
obj-$(CONFIG_CLK_PX30) += clk-px30.o
diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
index 29f31a5526fa..1bf84ac44e85 100644
--- a/drivers/clk/rockchip/clk-rk3588.c
+++ b/drivers/clk/rockchip/clk-rk3588.c
@@ -12,25 +12,6 @@
#include <dt-bindings/clock/rockchip,rk3588-cru.h>
#include "clk.h"
-/*
- * Recent Rockchip SoCs have a new hardware block called Native Interface
- * Unit (NIU), which gates clocks to devices behind them. These effectively
- * need two parent clocks.
- *
- * Downstream enables the linked clock via runtime PM whenever the gate is
- * enabled. This implementation uses separate clock nodes for each of the
- * linked gate clocks, which leaks parts of the clock tree into DT.
- *
- * The GATE_LINK macro instead takes the second parent via 'linkname', but
- * ignores the information. Once the clock framework is ready to handle it, the
- * information should be passed on here. But since these clocks are required to
- * access multiple relevant IP blocks, such as PCIe or USB, we mark all linked
- * clocks critical until a better solution is available. This will waste some
- * power, but avoids leaking implementation details into DT or hanging the
- * system.
- */
-#define GATE_LINK(_id, cname, pname, linkedclk, f, o, b, gf) \
- GATE(_id, cname, pname, f, o, b, gf)
#define RK3588_LINKED_CLK CLK_IS_CRITICAL
@@ -2513,8 +2494,8 @@ static int clk_rk3588_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
- rockchip_clk_register_branches(ctx, rk3588_clk_branches,
- ARRAY_SIZE(rk3588_clk_branches));
+ rockchip_clk_register_late_branches(dev, ctx, rk3588_clk_branches,
+ ARRAY_SIZE(rk3588_clk_branches));
rk3588_rst_init(np, ctx->reg_base);
rockchip_register_restart_notifier(ctx, RK3588_GLB_SRST_FST, NULL);
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index e150bc1fc319..f5f11cc60046 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -19,6 +19,7 @@
#include <linux/clk-provider.h>
#include <linux/io.h>
#include <linux/mfd/syscon.h>
+#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/reboot.h>
@@ -440,6 +441,29 @@ unsigned long rockchip_clk_find_max_clk_id(struct rockchip_clk_branch *list,
}
EXPORT_SYMBOL_GPL(rockchip_clk_find_max_clk_id);
+static struct platform_device *rockchip_clk_register_gate_link(
+ struct device *parent_dev,
+ struct rockchip_clk_provider *ctx,
+ struct rockchip_clk_branch *clkbr)
+{
+ struct rockchip_gate_link_platdata gate_link_pdata = {
+ .ctx = ctx,
+ .clkbr = clkbr,
+ };
+
+ struct platform_device_info pdevinfo = {
+ .parent = parent_dev,
+ .name = "rockchip-gate-link-clk",
+ .id = clkbr->id,
+ .fwnode = dev_fwnode(parent_dev),
+ .of_node_reused = true,
+ .data = &gate_link_pdata,
+ .size_data = sizeof(gate_link_pdata),
+ };
+
+ return platform_device_register_full(&pdevinfo);
+}
+
void rockchip_clk_register_branches(struct rockchip_clk_provider *ctx,
struct rockchip_clk_branch *list,
unsigned int nr_clk)
@@ -565,6 +589,9 @@ void rockchip_clk_register_branches(struct rockchip_clk_provider *ctx,
list->div_width, list->div_flags,
ctx->reg_base, &ctx->lock);
break;
+ case branch_linked_gate:
+ /* must be registered late, fall-through for error message */
+ break;
}
/* none of the cases above matched */
@@ -585,6 +612,31 @@ void rockchip_clk_register_branches(struct rockchip_clk_provider *ctx,
}
EXPORT_SYMBOL_GPL(rockchip_clk_register_branches);
+void rockchip_clk_register_late_branches(struct device *dev,
+ struct rockchip_clk_provider *ctx,
+ struct rockchip_clk_branch *list,
+ unsigned int nr_clk)
+{
+ unsigned int idx;
+
+ for (idx = 0; idx < nr_clk; idx++, list++) {
+ struct platform_device *pdev = NULL;
+
+ switch (list->branch_type) {
+ case branch_linked_gate:
+ pdev = rockchip_clk_register_gate_link(dev, ctx, list);
+ break;
+ default:
+ dev_err(dev, "unknown clock type %d\n", list->branch_type);
+ break;
+ }
+
+ if (!pdev)
+ dev_err(dev, "failed to register device for clock %s\n", list->name);
+ }
+}
+EXPORT_SYMBOL_GPL(rockchip_clk_register_late_branches);
+
void rockchip_clk_register_armclk(struct rockchip_clk_provider *ctx,
unsigned int lookup_id,
const char *name, const char *const *parent_names,
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index e39392e1c2a2..15aa2fd5265b 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -517,6 +517,7 @@ enum rockchip_clk_branch_type {
branch_divider,
branch_fraction_divider,
branch_gate,
+ branch_linked_gate,
branch_mmc,
branch_inverter,
branch_factor,
@@ -544,6 +545,7 @@ struct rockchip_clk_branch {
int gate_offset;
u8 gate_shift;
u8 gate_flags;
+ unsigned int linked_clk_id;
struct rockchip_clk_branch *child;
};
@@ -842,6 +844,20 @@ struct rockchip_clk_branch {
.gate_flags = gf, \
}
+#define GATE_LINK(_id, cname, pname, linkedclk, f, o, b, gf) \
+ { \
+ .id = _id, \
+ .branch_type = branch_linked_gate, \
+ .name = cname, \
+ .parent_names = (const char *[]){ pname }, \
+ .linked_clk_id = linkedclk, \
+ .num_parents = 1, \
+ .flags = f, \
+ .gate_offset = o, \
+ .gate_shift = b, \
+ .gate_flags = gf, \
+ }
+
#define MMC(_id, cname, pname, offset, shift) \
{ \
.id = _id, \
@@ -981,6 +997,11 @@ static inline void rockchip_clk_set_lookup(struct rockchip_clk_provider *ctx,
ctx->clk_data.clks[id] = clk;
}
+struct rockchip_gate_link_platdata {
+ struct rockchip_clk_provider *ctx;
+ struct rockchip_clk_branch *clkbr;
+};
+
struct rockchip_clk_provider *rockchip_clk_init(struct device_node *np,
void __iomem *base, unsigned long nr_clks);
void rockchip_clk_of_add_provider(struct device_node *np,
@@ -990,6 +1011,10 @@ unsigned long rockchip_clk_find_max_clk_id(struct rockchip_clk_branch *list,
void rockchip_clk_register_branches(struct rockchip_clk_provider *ctx,
struct rockchip_clk_branch *list,
unsigned int nr_clk);
+void rockchip_clk_register_late_branches(struct device *dev,
+ struct rockchip_clk_provider *ctx,
+ struct rockchip_clk_branch *list,
+ unsigned int nr_clk);
void rockchip_clk_register_plls(struct rockchip_clk_provider *ctx,
struct rockchip_pll_clock *pll_list,
unsigned int nr_pll, int grf_lock_offset);
diff --git a/drivers/clk/rockchip/gate-link.c b/drivers/clk/rockchip/gate-link.c
new file mode 100644
index 000000000000..47b6f3e7a6a2
--- /dev/null
+++ b/drivers/clk/rockchip/gate-link.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Collabora Ltd.
+ * Author: Sebastian Reichel <sebastian.reichel@collabora.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include "clk.h"
+
+static int rk_clk_gate_link_register(struct device *dev,
+ struct rockchip_clk_provider *ctx,
+ struct rockchip_clk_branch *clkbr)
+{
+ unsigned long flags = clkbr->flags | CLK_SET_RATE_PARENT;
+ struct clk *clk;
+
+ clk = clk_register_gate(dev, clkbr->name, clkbr->parent_names[0],
+ flags, ctx->reg_base + clkbr->gate_offset,
+ clkbr->gate_shift, clkbr->gate_flags,
+ &ctx->lock);
+
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ rockchip_clk_set_lookup(ctx, clk, clkbr->id);
+ return 0;
+}
+
+static int rk_clk_gate_link_probe(struct platform_device *pdev)
+{
+ struct rockchip_gate_link_platdata *pdata;
+ struct device *dev = &pdev->dev;
+ struct clk *linked_clk;
+ int ret;
+
+ pdata = dev_get_platdata(dev);
+ if (!pdata)
+ return dev_err_probe(dev, -ENODEV, "missing platform data");
+
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ ret = devm_pm_clk_create(dev);
+ if (ret)
+ return ret;
+
+ linked_clk = rockchip_clk_get_lookup(pdata->ctx, pdata->clkbr->linked_clk_id);
+ ret = pm_clk_add_clk(dev, linked_clk);
+ if (ret)
+ return ret;
+
+ ret = rk_clk_gate_link_register(dev, pdata->ctx, pdata->clkbr);
+ if (ret)
+ goto err;
+
+ return 0;
+
+err:
+ pm_clk_remove_clk(dev, linked_clk);
+ return ret;
+}
+
+static void rk_clk_gate_link_remove(struct platform_device *pdev)
+{
+ struct rockchip_gate_link_platdata *pdata;
+ struct device *dev = &pdev->dev;
+ struct clk *clk, *linked_clk;
+
+ pdata = dev_get_platdata(dev);
+ clk = rockchip_clk_get_lookup(pdata->ctx, pdata->clkbr->id);
+ linked_clk = rockchip_clk_get_lookup(pdata->ctx, pdata->clkbr->linked_clk_id);
+ rockchip_clk_set_lookup(pdata->ctx, ERR_PTR(-ENODEV), pdata->clkbr->id);
+ clk_unregister_gate(clk);
+ pm_clk_remove_clk(dev, linked_clk);
+}
+
+static const struct dev_pm_ops rk_clk_gate_link_pm_ops = {
+ SET_RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
+struct platform_driver rk_clk_gate_link_driver = {
+ .probe = rk_clk_gate_link_probe,
+ .remove_new = rk_clk_gate_link_remove,
+ .driver = {
+ .name = "rockchip-gate-link-clk",
+ .pm = &rk_clk_gate_link_pm_ops,
+ },
+};
+
+static int __init rk_clk_gate_link_drv_register(void)
+{
+ return platform_driver_register(&rk_clk_gate_link_driver);
+}
+core_initcall(rk_clk_gate_link_drv_register);
--
2.43.0
^ permalink raw reply related
* [PATCH v9 5/7] clk: rockchip: fix error for unknown clocks
From: Sebastian Reichel @ 2024-03-25 19:33 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, linux-clk
Cc: Elaine Zhang, Kever Yang, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, huangtao, andy.yan,
Michal Tomek, Ilya K, Chad LeClair, devicetree, linux-rockchip,
Sebastian Reichel, kernel
In-Reply-To: <20240325193609.237182-1-sebastian.reichel@collabora.com>
There is a clk == NULL check after the switch to check for
unsupported clk types. Since clk is re-assigned in a loop,
this check is useless right now for anything but the first
round. Let's fix this up by assigning clk = NULL in the
loop before the switch statement.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/clk/rockchip/clk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index ef2408f10f39..e150bc1fc319 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -444,12 +444,13 @@ void rockchip_clk_register_branches(struct rockchip_clk_provider *ctx,
struct rockchip_clk_branch *list,
unsigned int nr_clk)
{
- struct clk *clk = NULL;
+ struct clk *clk;
unsigned int idx;
unsigned long flags;
for (idx = 0; idx < nr_clk; idx++, list++) {
flags = list->flags;
+ clk = NULL;
/* catch simple muxes */
switch (list->branch_type) {
--
2.43.0
^ 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