* [PATCH 01/10] dt-bindings: riscv: add Zca, Zcf, Zcd and Zcb ISA extension description
From: Clément Léger @ 2024-04-10 9:10 UTC (permalink / raw)
To: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Conor Dooley, Rob Herring, Krzysztof Kozlowski, Anup Patel,
Shuah Khan
Cc: Clément Léger, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <20240410091106.749233-1-cleger@rivosinc.com>
Add description for Zca, Zcf, Zcd and Zcb extensions which are part the
Zc* standard extensions for code size reduction.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
.../devicetree/bindings/riscv/extensions.yaml | 32 +++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
index 616370318a66..516f57bdfeeb 100644
--- a/Documentation/devicetree/bindings/riscv/extensions.yaml
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -220,6 +220,38 @@ properties:
instructions as ratified at commit 6d33919 ("Merge pull request #158
from hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
+ - const: zca
+ description: |
+ The Zca extension part of Zc* standard extensions for code size
+ reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on
+ RV64 as it contains no instructions") of riscv-code-size-reduction,
+ merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed
+ of zc.adoc to src tree.").
+
+ - const: zcb
+ description: |
+ The Zcb extension part of Zc* standard extensions for code size
+ reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on
+ RV64 as it contains no instructions") of riscv-code-size-reduction,
+ merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed
+ of zc.adoc to src tree.").
+
+ - const: zcd
+ description: |
+ The Zcd extension part of Zc* standard extensions for code size
+ reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on
+ RV64 as it contains no instructions") of riscv-code-size-reduction,
+ merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed
+ of zc.adoc to src tree.").
+
+ - const: zcf
+ description: |
+ The Zcf extension part of Zc* standard extensions for code size
+ reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on
+ RV64 as it contains no instructions") of riscv-code-size-reduction,
+ merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed
+ of zc.adoc to src tree.").
+
- const: zfa
description:
The standard Zfa extension for additional floating point
--
2.43.0
^ permalink raw reply related
* [PATCH 00/10] Add support for a few Zc* extensions as well as Zcmop
From: Clément Léger @ 2024-04-10 9:10 UTC (permalink / raw)
To: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Conor Dooley, Rob Herring, Krzysztof Kozlowski, Anup Patel,
Shuah Khan
Cc: Clément Léger, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
Add support for (yet again) more RVA23U64 missing extensions. Add
support for Zcmop, Zca, Zcf, Zcd and Zcb extensions isa string parsing,
hwprobe and kvm support. Zce, Zcmt and Zcmp extensions have been left
out since they target microcontrollers/embedded CPUs and are not needed
by RVA23U64
This series is based on the Zimop one [1].
Link: https://lore.kernel.org/linux-riscv/20240404103254.1752834-1-cleger@rivosinc.com/ [1]
Clément Léger (10):
dt-bindings: riscv: add Zca, Zcf, Zcd and Zcb ISA extension
description
riscv: add ISA parsing for Zca, Zcf, Zcd and Zcb
riscv: hwprobe: export Zca, Zcf, Zcd and Zcb ISA extensions
RISC-V: KVM: Allow Zca, Zcf, Zcd and Zcb extensions for Guest/VM
KVM: riscv: selftests: Add some Zc* extensions to get-reg-list test
dt-bindings: riscv: add Zcmop ISA extension description
riscv: add ISA extension parsing for Zcmop
riscv: hwprobe: export Zcmop ISA extension
RISC-V: KVM: Allow Zcmop extension for Guest/VM
KVM: riscv: selftests: Add Zcmop extension to get-reg-list test
Documentation/arch/riscv/hwprobe.rst | 24 ++++++++++++
.../devicetree/bindings/riscv/extensions.yaml | 37 +++++++++++++++++++
arch/riscv/include/asm/hwcap.h | 5 +++
arch/riscv/include/uapi/asm/hwprobe.h | 5 +++
arch/riscv/include/uapi/asm/kvm.h | 5 +++
arch/riscv/kernel/cpufeature.c | 5 +++
arch/riscv/kernel/sys_hwprobe.c | 5 +++
arch/riscv/kvm/vcpu_onereg.c | 10 +++++
.../selftests/kvm/riscv/get-reg-list.c | 20 ++++++++++
9 files changed, 116 insertions(+)
--
2.43.0
^ permalink raw reply
* Re: [RFC PATCH v2 1/5] clk: meson: axg: move reset controller's code to separate module
From: Philipp Zabel @ 2024-04-10 8:56 UTC (permalink / raw)
To: Stephen Boyd, Conor Dooley
Cc: Jan Dakinevich, Jerome Brunet, Neil Armstrong, Michael Turquette,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <dde59dd2ef4da81528e31f65844e0b3f.sboyd@kernel.org>
On Di, 2024-04-09 at 19:27 -0700, Stephen Boyd wrote:
> Quoting Conor Dooley (2024-04-09 05:05:37)
> > On Mon, Apr 08, 2024 at 06:05:51PM +0100, Conor Dooley wrote:
> >
> > > > > Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> > > > > starfive-jh7110 drivers are examples of this.
> > > > >
> > > > > > The auxiliary device creation function can also be in the
> > > > > > drivers/reset/ directory so that the clk driver calls some function
> > > > > > to create and register the device.
> > > > >
> > > > > I'm undecided about this, do you think mpfs_reset_controller_register()
> > > > > and jh7110_reset_controller_register() should rather live with the
> > > > > reset aux drivers in drivers/reset/ ?
> > > >
> > > > Yes, and also mpfs_reset_read() and friends. We should pass the base
> > > > iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
> > > > then move all that code into drivers/reset with some header file
> > > > exported function to call. That way the clk driver hands over the data
> > > > without having to implement half the implementation.
> > >
> > > I'll todo list that :)
> >
> > Something like the below?
> >
> > -- >8 --
> > From a12f281d2cb869bcd9a6ffc45d0c6a0d3aa2e9e2 Mon Sep 17 00:00:00 2001
> > From: Conor Dooley <conor.dooley@microchip.com>
> > Date: Tue, 9 Apr 2024 11:54:34 +0100
> > Subject: [PATCH] clock, reset: microchip: move all mpfs reset code to the
> > reset subsystem
> >
> > <insert something here>
> >
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>
> Looks pretty good.
Yes, that does look convincing.
regards
Philipp
^ permalink raw reply
* Re: (subset) [PATCH v12 0/7] drm/meson: add support for MIPI DSI Display
From: Neil Armstrong @ 2024-04-10 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Martin Blumenstingl, Jerome Brunet, Kevin Hilman,
Michael Turquette, Stephen Boyd, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Jagan Teki,
Nicolas Belin, Neil Armstrong
Cc: devicetree, linux-kernel, linux-amlogic, linux-clk,
linux-arm-kernel, dri-devel, Conor Dooley, Lukas F. Hartmann
In-Reply-To: <20240403-amlogic-v6-4-upstream-dsi-ccf-vim3-v12-0-99ecdfdc87fc@linaro.org>
Hi,
On Wed, 03 Apr 2024 09:46:31 +0200, Neil Armstrong wrote:
> The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a),
> with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI
> glue on the same Amlogic SoCs.
>
> This is a follow-up of v5 now the DRM patches are applied, the clk & DT changes
> remains for a full DSI support on G12A & SM1 platforms.
>
> [...]
Thanks, Applied to https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git (v6.10/arm64-dt)
[1/7] dt-bindings: arm: amlogic: Document the MNT Reform 2 CM4 adapter with a BPI-CM4 Module
https://git.kernel.org/amlogic/c/ef5a84d716042871599ff7c8ff571a6390b99718
[5/7] arm64: meson: g12-common: add the MIPI DSI nodes
https://git.kernel.org/amlogic/c/6f1c2a12ed1138c3e680935718672d361afee372
[6/7] arm64: meson: khadas-vim3l: add TS050 DSI panel overlay
https://git.kernel.org/amlogic/c/2a885bad5ba4d553758d3f1689000cee8e6dae87
[7/7] arm64: dts: amlogic: meson-g12b-bananapi-cm4: add support for MNT Reform2 with CM4 adaper
https://git.kernel.org/amlogic/c/fde2d69c1626bebb3a8851909c912e582db1ca95
These changes has been applied on the intermediate git tree [1].
The v6.10/arm64-dt branch will then be sent via a formal Pull Request to the Linux SoC maintainers
for inclusion in their intermediate git branches in order to be sent to Linus during
the next merge window, or sooner if it's a set of fixes.
In the cases of fixes, those will be merged in the current release candidate
kernel and as soon they appear on the Linux master branch they will be
backported to the previous Stable and Long-Stable kernels [2].
The intermediate git branches are merged daily in the linux-next tree [3],
people are encouraged testing these pre-release kernels and report issues on the
relevant mailing-lists.
If problems are discovered on those changes, please submit a signed-off-by revert
patch followed by a corrective changeset.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git
[2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
[3] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
--
Neil
^ permalink raw reply
* Re: [PATCH v11 0/7] drm/meson: add support for MIPI DSI Display
From: Neil Armstrong @ 2024-04-10 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kevin Hilman,
Jerome Brunet, Michael Turquette, Stephen Boyd,
Martin Blumenstingl, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Nicolas Belin,
Jagan Teki, Neil Armstrong
Cc: devicetree, linux-kernel, linux-amlogic, linux-clk,
linux-arm-kernel, dri-devel, Conor Dooley, Lukas F. Hartmann
In-Reply-To: <20240325-amlogic-v6-4-upstream-dsi-ccf-vim3-v11-0-04f55de44604@linaro.org>
Hi,
On Mon, 25 Mar 2024 12:09:46 +0100, Neil Armstrong wrote:
> The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a),
> with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI
> glue on the same Amlogic SoCs.
>
> This is a follow-up of v5 now the DRM patches are applied, the clk & DT changes
> remains for a full DSI support on G12A & SM1 platforms.
>
> [...]
Thanks, Applied to https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git (v6.10/arm64-dt)
[1/7] dt-bindings: arm: amlogic: Document the MNT Reform 2 CM4 adapter with a BPI-CM4 Module
https://git.kernel.org/amlogic/c/ef5a84d716042871599ff7c8ff571a6390b99718
[2/7] clk: meson: add vclk driver
(no commit info)
[3/7] clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF
(no commit info)
[4/7] drm/meson: gate px_clk when setting rate
(no commit info)
[5/7] arm64: meson: g12-common: add the MIPI DSI nodes
https://git.kernel.org/amlogic/c/6f1c2a12ed1138c3e680935718672d361afee372
[6/7] arm64: meson: khadas-vim3l: add TS050 DSI panel overlay
https://git.kernel.org/amlogic/c/2a885bad5ba4d553758d3f1689000cee8e6dae87
[7/7] arm64: dts: amlogic: meson-g12b-bananapi-cm4: add support for MNT Reform2 with CM4 adaper
https://git.kernel.org/amlogic/c/fde2d69c1626bebb3a8851909c912e582db1ca95
These changes has been applied on the intermediate git tree [1].
The v6.10/arm64-dt branch will then be sent via a formal Pull Request to the Linux SoC maintainers
for inclusion in their intermediate git branches in order to be sent to Linus during
the next merge window, or sooner if it's a set of fixes.
In the cases of fixes, those will be merged in the current release candidate
kernel and as soon they appear on the Linux master branch they will be
backported to the previous Stable and Long-Stable kernels [2].
The intermediate git branches are merged daily in the linux-next tree [3],
people are encouraged testing these pre-release kernels and report issues on the
relevant mailing-lists.
If problems are discovered on those changes, please submit a signed-off-by revert
patch followed by a corrective changeset.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git
[2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
[3] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
--
Neil
^ permalink raw reply
* Re: [PATCH v7 3/3] pinctrl: nuvoton: Add ma35d1 pinctrl and GPIO driver
From: Andy Shevchenko @ 2024-04-10 8:54 UTC (permalink / raw)
To: Jacky Huang
Cc: linus.walleij, robh+dt, krzysztof.kozlowski+dt, conor+dt, p.zabel,
j.neuschaefer, linux-arm-kernel, linux-gpio, devicetree,
linux-kernel, ychuang3, schung
In-Reply-To: <20240409095637.2135-4-ychuang570808@gmail.com>
Tue, Apr 09, 2024 at 09:56:37AM +0000, Jacky Huang kirjoitti:
> From: Jacky Huang <ychuang3@nuvoton.com>
>
> Add common pinctrl and GPIO driver for Nuvoton MA35 series SoC, and
> add support for ma35d1 pinctrl.
...
> + * Copyright (C) 2023 Nuvoton Technology Corp.
Almos mid of 2024...
...
> +#include <linux/gpio/driver.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
Do you really need all of these pf*.h?
Don't you miss other headers to be included (spoiler: a lot!)
> +#include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinctrl.h>
Move this group...
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
...to be here to show the relation to the pin control subsystem.
> +#include "../core.h"
> +#include "../pinconf.h"
> +#include "pinctrl-ma35.h"
...
> +#define MA35_GP_MODE_MASK_WIDTH 2
> +
> +#define MA35_GP_SLEWCTL_MASK_WIDTH 2
I looked at the code how you use these... Oh, please switch to FIELD_GET() /
FIELD_PREP() (don't forget to include bitfield.h)
...
> +struct ma35_pin_func {
> + const char *name;
> + const char **groups;
> + u32 ngroups;
> +};
NIH struct pinfunction.
You may still have a wrapper (as struct pingroup below most likely will be
embedded in your custom data type), see the pinctrl-intel.h examples.
...
> +struct ma35_pin_group {
> + const char *name;
> + unsigned int npins;
> + unsigned int *pins;
NIH struct pingroup.
> + struct ma35_pin_setting *settings;
> +};
...
> +struct ma35_pin_bank {
> + void __iomem *reg_base;
> + struct clk *clk;
> + int irq;
> + u8 nr_pins;
> + const char *name;
> + u8 bank_num;
Interleaved fields with such a different type may lead to waste of memory. Run
`pahole` and update the ordering in this struct accordingly.
> + bool valid;
> + struct device_node *np;
Can you keep it fwnode-based?
> + struct gpio_chip chip;
> + u32 irqtype;
> + u32 irqinten;
> + struct regmap *regmap;
> + struct device *dev;
> + spinlock_t lock;
No use in RT_PREEMPT?
> +};
...
> + new_map = devm_kcalloc(pctldev->dev, map_num, sizeof(*new_map), GFP_KERNEL);
> + if (!new_map)
> + return -ENOMEM;
> +
> + *map = new_map;
> + *num_maps = map_num;
> + /* create mux map */
> + parent = of_get_parent(np);
> + if (!parent) {
> + devm_kfree(pctldev->dev, new_map);
Huh?! Are you sure you know the scope of usage of devm_*()?
> + return -EINVAL;
> + }
> +
...
> + regval &= ~GENMASK(setting->shift + MA35_MFP_BITS_PER_PORT - 1,
> + setting->shift);
This will generate an awful code. Use respective FIELD_*() macros.
...
> + regval &= ~GENMASK(gpio * MA35_GP_MODE_MASK_WIDTH - 1,
> + gpio * MA35_GP_MODE_MASK_WIDTH);
> + regval |= mode << gpio * MA35_GP_MODE_MASK_WIDTH;
Ditto.
...
> + regval &= GENMASK(gpio * MA35_GP_MODE_MASK_WIDTH - 1,
> + gpio * MA35_GP_MODE_MASK_WIDTH);
> +
> + return regval >> gpio * MA35_GP_MODE_MASK_WIDTH;
Ditto.
...
> +static int ma35_gpio_core_direction_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct ma35_pin_bank *bank = gpiochip_get_data(gc);
> + void __iomem *reg_mode = bank->reg_base + MA35_GP_REG_MODE;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&bank->lock, flags);
Use cleanup.h, i.e. guard() in this case, from day 1.
Similar approach for all other places.
> + spin_unlock_irqrestore(&bank->lock, flags);
> +
> + return 0;
> +}
...
> + regval = readl(reg_dout);
> + if (val)
> + writel(regval | BIT(gpio), reg_dout);
> + else
> + writel(regval & ~BIT(gpio), reg_dout);
Can you split regval update and make only a single writel() call? It's slightly
better pattern
read()
if (foo)
v =
else
v =
write()
> + ma35_gpio_set_mode(reg_mode, gpio, MA35_GP_MODE_OUTPUT);
...
> + regval &= ~GENMASK(bit_offs + MA35_MFP_BITS_PER_PORT - 1, bit_offs);
FIELD_GET()
...
> + writel(1<<num, reg_intsrc);
Oh, something happened here, besides indentation.
Have you meant BIT() ?
...
> + unsigned int num = (d->hwirq);
Read Documentation on how to access hwirq field and what type of the value
should be. There are plently of the examples already in the kernel. Just
check the recent (more or less) ones.
...
> + /*
> + * The MA35_GP_REG_INTEN bits 0 ~ 15 control low-level or falling edge trigger,
> + * while bits 16 ~ 31 control high-level or rising edge trigger.
> + * We disable both type of interrupt.
> + */
> + regval &= ~(BIT(num + 16) | BIT(num));
You have this idiom more than once in the code, make the definition and use it.
Move comment closer to that definition.
...
> + irq_set_handler_locked(d, handle_edge_irq);
> + bank->irqtype &= ~(0x1 << num);
> + bank->irqinten |= (0x1 << num);
> + bank->irqinten &= ~(0x1 << (num + 16));
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + irq_set_handler_locked(d, handle_level_irq);
> + bank->irqtype |= (0x1 << num);
> + bank->irqinten &= ~(0x1 << num);
> + bank->irqinten |= (0x1 << (num + 16));
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + irq_set_handler_locked(d, handle_level_irq);
> + bank->irqtype |= (0x1 << num);
> + bank->irqinten &= ~(0x1 << (num + 16));
> + bank->irqinten |= (0x1 << num);
> + break;
BIT() in all cases.
...
> + irq_set_handler_locked(d, handle_bad_irq);
Just assign it in the probe.
...
> +static struct irq_chip ma35_gpio_irqchip = {
> + .name = "MA35-GPIO-IRQ",
> + .irq_disable = ma35_irq_gpio_mask,
> + .irq_enable = ma35_irq_gpio_unmask,
> + .irq_ack = ma35_irq_gpio_ack,
> + .irq_mask = ma35_irq_gpio_mask,
> + .irq_unmask = ma35_irq_gpio_unmask,
> + .irq_set_type = ma35_irq_irqtype,
> + .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE | IRQCHIP_IMMUTABLE,
It doesn't look like IMMUTABLE. Again, check other examples, there are a lot.
...
> +static void ma35_irq_demux_intgroup(struct irq_desc *desc)
> +{
> + struct ma35_pin_bank *bank = gpiochip_get_data(irq_desc_get_handler_data(desc));
> + struct irq_domain *irqdomain = bank->chip.irq.domain;
> + struct irq_chip *irqchip = irq_desc_get_chip(desc);
> + unsigned long isr;
> + int offset;
> +
> + chained_irq_enter(irqchip, desc);
> + isr = readl(bank->reg_base + MA35_GP_REG_INTSRC);
> + if (isr)
Unneeded duplicate check.
> + for_each_set_bit(offset, &isr, bank->nr_pins)
> + generic_handle_irq(irq_find_mapping(irqdomain, offset));
> +
> + chained_irq_exit(irqchip, desc);
> +}
...
> + for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
Why pre-increments?
> + if (!bank->valid) {
> + dev_warn(&pdev->dev, "bank %s is not valid\n",
> + bank->np->name);
Do not use fwnode / of_node name like this, use proper specifiers: %pfw / %pOF.
> + continue;
> + }
> + bank->irqtype = 0;
> + bank->irqinten = 0;
> + bank->chip.label = bank->name;
> + bank->chip.of_gpio_n_cells = 2;
> + bank->chip.parent = &pdev->dev;
> + bank->chip.request = ma35_gpio_core_to_request;
> + bank->chip.direction_input = ma35_gpio_core_direction_in;
> + bank->chip.direction_output = ma35_gpio_core_direction_out;
> + bank->chip.get = ma35_gpio_core_get;
> + bank->chip.set = ma35_gpio_core_set;
> + bank->chip.base = -1;
> + bank->chip.ngpio = bank->nr_pins;
> + bank->chip.can_sleep = false;
> + spin_lock_init(&bank->lock);
> +
> + if (bank->irq > 0) {
> + struct gpio_irq_chip *girq;
> +
> + girq = &bank->chip.irq;
> + gpio_irq_chip_set_chip(girq, &ma35_gpio_irqchip);
> + girq->parent_handler = ma35_irq_demux_intgroup;
> + girq->num_parents = 1;
> +
> + girq->parents = devm_kcalloc(&pdev->dev, 1, sizeof(*girq->parents),
Use num_parents instead of 1.
> + GFP_KERNEL);
> + if (!girq->parents) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + girq->parents[0] = bank->irq;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_bad_irq;
> + }
> +
> + ret = gpiochip_add_data(&bank->chip, bank);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register gpio_chip %s, error code: %d\n",
> + bank->chip.label, ret);
> + goto fail;
> + }
> + }
...
> +fail:
> + for (--i, --bank; i >= 0; --i, --bank) {
while (i--) {
bank--;
...
}
much better to read.
> + if (!bank->valid)
> + continue;
> + gpiochip_remove(&bank->chip);
> + }
> + return ret;
> +}
...
> +static int ma35_get_bank_data(struct ma35_pin_bank *bank, struct ma35_pinctrl *npctl)
> +{
> + struct resource res;
> +
> + if (of_address_to_resource(bank->np, 0, &res)) {
> + dev_err(npctl->dev, "cannot find IO resource for bank\n");
> + return -ENOENT;
> + }
> +
> + bank->reg_base = devm_ioremap_resource(npctl->dev, &res);
> + if (IS_ERR(bank->reg_base)) {
> + dev_err(npctl->dev, "cannot ioremap resource for bank\n");
> + return PTR_ERR(bank->reg_base);
> + }
Use fwnode_iomap()
> + bank->irq = irq_of_parse_and_map(bank->np, 0);
Use fwnode_get_irq()
> + bank->nr_pins = MA35_GPIO_PORT_MAX;
> +
> + bank->clk = of_clk_get(bank->np, 0);
Can't you use simple clk_get()? Why?
> + if (IS_ERR(bank->clk))
> + return PTR_ERR(bank->clk);
> +
> + return clk_prepare_enable(bank->clk);
> +}
...
> + for_each_gpiochip_node(&pdev->dev, child) {
> + struct device_node *np = to_of_node(child);
No need, just keep everythin fwnode based.
> + bank = &ctrl->pin_banks[id];
> + bank->np = np;
> + bank->regmap = pctl->regmap;
> + bank->dev = &pdev->dev;
> + if (!ma35_get_bank_data(bank, pctl))
> + bank->valid = true;
> + id++;
> + }
...
> + regval &= ~GENMASK(port * MA35_GP_PUSEL_MASK_WIDTH - 1,
> + port * MA35_GP_PUSEL_MASK_WIDTH);
FIELD_*()
...
> + regval &= GENMASK(port * MA35_GP_PUSEL_MASK_WIDTH - 1,
> + port * MA35_GP_PUSEL_MASK_WIDTH);
> + regval >>= port * MA35_GP_PUSEL_MASK_WIDTH;
Ditto.
...
> + if (regval & BIT(port))
> + return MVOLT_3300;
> + else
> + return MVOLT_1800;
Can be written as
return (regval & BIT()) ? _3300 : _1800;
...
> + if (port < MA35_GP_DSH_BASE_PORT) {
> + regval = readl(base + MA35_GP_REG_DSL);
> + ds_val = (regval & GENMASK(port * 4 + 2, port * 4)) >> port * 4;
This is too complicated way of saying
ds_val = (regval >> port * 4) & GENMASK(2, 0);
> + } else {
> + port -= MA35_GP_DSH_BASE_PORT;
> + regval = readl(base + MA35_GP_REG_DSH);
> + ds_val = (regval & GENMASK(port * 4 + 2, port * 4)) >> port * 4;
Ditto.
> + }
...
> + if (ma35_pinconf_get_power_source(npctl, pin) == MVOLT_1800) {
> + for (i = 0; i < ARRAY_SIZE(ds_1800mv_tbl); i++) {
> + if (ds_1800mv_tbl[i] == strength)
> + ds_val = i;
And still continue?!
> + }
> + } else {
> + for (i = 0; i < ARRAY_SIZE(ds_3300mv_tbl); i++) {
> + if (ds_3300mv_tbl[i] == strength)
> + ds_val = i;
Ditto.
> + }
> + }
Wondering if the linear_ranges or other existing tables approaches can be used here.
...
> + if (port < MA35_GP_DSH_BASE_PORT) {
> + regval = readl(base + MA35_GP_REG_DSL);
> + regval &= ~GENMASK(port * 4 + 2, port * 4);
> + regval |= ds_val << port * 4;
As per above
~(GENMASK(2, 0) << (port * 4))
> + writel(regval, base + MA35_GP_REG_DSL);
> + } else {
> + port -= MA35_GP_DSH_BASE_PORT;
> + regval = readl(base + MA35_GP_REG_DSH);
> + regval &= ~GENMASK(port * 4 + 2, port * 4);
Ditto.
> + regval |= ds_val << port * 4;
> + writel(regval, base + MA35_GP_REG_DSH);
> + }
...
> + regval &= ~GENMASK(port * MA35_GP_SLEWCTL_MASK_WIDTH - 1,
> + port * MA35_GP_SLEWCTL_MASK_WIDTH);
> + regval |= rate << port * MA35_GP_SLEWCTL_MASK_WIDTH;
FIELD_*()
...
> +static int ma35_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin, unsigned long *config)
> +{
> + struct ma35_pinctrl *npctl = pinctrl_dev_get_drvdata(pctldev);
> + enum pin_config_param param = pinconf_to_config_param(*config);
> + u32 arg;
> + int ret;
> +
> + switch (param) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + case PIN_CONFIG_BIAS_PULL_UP:
> + if (ma35_pinconf_get_pull(npctl, pin) == param)
> + arg = 1;
> + else
> + return -EINVAL;
> + break;
Check for the error case first and get rid of redundant 'else'.
> + case PIN_CONFIG_DRIVE_STRENGTH:
> + ret = ma35_pinconf_get_drive_strength(npctl, pin, &arg);
> + if (ret)
> + return ret;
> + break;
> +
> + case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> + arg = ma35_pinconf_get_schmitt_enable(npctl, pin);
> + break;
> +
> + case PIN_CONFIG_SLEW_RATE:
> + arg = ma35_pinconf_get_slew_rate(npctl, pin);
> + break;
> +
> + case PIN_CONFIG_OUTPUT_ENABLE:
> + arg = ma35_pinconf_get_output(npctl, pin);
> + break;
> +
> + case PIN_CONFIG_POWER_SOURCE:
> + arg = ma35_pinconf_get_power_source(npctl, pin);
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> + *config = pinconf_to_config_packed(param, arg);
> +
> + return 0;
> +}
...
> +static int ma35_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> + unsigned long *configs, unsigned int num_configs)
> +{
> + struct ma35_pinctrl *npctl = pinctrl_dev_get_drvdata(pctldev);
> + enum pin_config_param param;
> + unsigned int arg = 0;
> + int i, ret = 0;
> +
> + for (i = 0; i < num_configs; i++) {
> + param = pinconf_to_config_param(configs[i]);
> + arg = pinconf_to_config_argument(configs[i]);
> +
> + switch (param) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + case PIN_CONFIG_BIAS_PULL_UP:
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + ret = ma35_pinconf_set_pull(npctl, pin, param);
> + break;
> +
> + case PIN_CONFIG_DRIVE_STRENGTH:
> + ret = ma35_pinconf_set_drive_strength(npctl, pin, arg);
> + break;
> +
> + case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> + ret = ma35_pinconf_set_schmitt(npctl, pin, 1);
> + break;
> +
> + case PIN_CONFIG_INPUT_SCHMITT:
> + ret = ma35_pinconf_set_schmitt(npctl, pin, arg);
> + break;
> +
> + case PIN_CONFIG_SLEW_RATE:
> + ret = ma35_pinconf_set_slew_rate(npctl, pin, arg);
> + break;
> +
> + case PIN_CONFIG_OUTPUT_ENABLE:
> + ret = ma35_pinconf_set_output(npctl, pin, arg);
> + break;
> +
> + case PIN_CONFIG_POWER_SOURCE:
> + ret = ma35_pinconf_set_power_source(npctl, pin, arg);
> + break;
> +
> + default:
> + return -EINVAL;
> + }
No returning an error if we got ret != 0?!
> + }
> + return ret;
> +}
...
> +static int ma35_pinctrl_parse_groups(struct device_node *np, struct ma35_pin_group *grp,
> + struct ma35_pinctrl *npctl, u32 index)
> +{
> + unsigned long *configs;
> + unsigned int nconfigs;
> + struct ma35_pin_setting *pin;
> + const __be32 *list;
Huh?!
> + int i, j, size, ret;
> +
> + dev_dbg(npctl->dev, "group(%d): %s\n", index, np->name);
> +
> + grp->name = np->name;
> +
> + ret = pinconf_generic_parse_dt_config(np, NULL, &configs, &nconfigs);
> + if (ret)
> + return ret;
> +
> + /*
> + * the binding format is nuvoton,pins = <bank pin-mfp pin-function>,
> + * do sanity check and calculate pins number
> + */
/*
* Respect English grammar and puntuation
* in all multi-line comments. See the difference
* how it's done above and in this example.
*/
> + list = of_get_property(np, "nuvoton,pins", &size);
Oh, no.
Use respective of_property_ calls.
> + size /= sizeof(*list);
> + if (!size || size % 3) {
> + dev_err(npctl->dev, "wrong setting!\n");
> + return -EINVAL;
> + }
> + grp->npins = size / 3;
> +
> + grp->pins = devm_kcalloc(npctl->dev, grp->npins, sizeof(*grp->pins), GFP_KERNEL);
> + if (!grp->pins)
> + return -ENOMEM;
> +
> + grp->settings = devm_kcalloc(npctl->dev, grp->npins, sizeof(*grp->settings), GFP_KERNEL);
> + if (!grp->settings)
> + return -ENOMEM;
> +
> + pin = grp->settings;
> +
> + for (i = 0, j = 0; i < size; i += 3, j++) {
> + pin->offset = be32_to_cpu(*list++) * MA35_MFP_REG_SZ_PER_BANK + MA35_MFP_REG_BASE;
> + pin->shift = (be32_to_cpu(*list++) * MA35_MFP_BITS_PER_PORT) % 32;
> + pin->muxval = be32_to_cpu(*list++);
> + pin->configs = configs;
> + pin->nconfigs = nconfigs;
> + grp->pins[j] = npctl->info->get_pin_num(pin->offset, pin->shift);
> + pin++;
> + }
> + return 0;
> +}
...
> + for_each_child_of_node(np, child) {
> + if (of_property_read_bool(child, "gpio-controller"))
> + continue;
There is a macro already present, for_each_gpiochip_node().
> + npctl->nfunctions++;
> + npctl->ngroups += of_get_child_count(child);
> + }
...
> + for_each_child_of_node(np, child) {
> + if (of_property_read_bool(child, "gpio-controller"))
> + continue;
Ditto.
> + ret = ma35_pinctrl_parse_functions(child, npctl, idx++);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to parse function\n");
> + of_node_put(child);
> + return ret;
> + }
> + }
...
> + if (!info || !info->pins || !info->npins) {
> + dev_err(&pdev->dev, "wrong pinctrl info\n");
> + return -EINVAL;
return dev_err_probe(...);
> + }
...
> + ret = devm_pinctrl_register_and_init(&pdev->dev, ma35_pinctrl_desc,
> + npctl, &npctl->pctl);
Haveing
struct device *dev = &pdev->dev;
makes in particular this one better
ret = devm_pinctrl_register_and_init(dev, ma35_pinctrl_desc, npctl, &npctl->pctl);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "fail to register MA35 pinctrl\n");
...
> +int __maybe_unused ma35_pinctrl_suspend(struct device *dev)
No __maybe_unused.
Just use respective PM macros (pm_ptr(), DEFINE_,*PM_OPS, etc).
> +{
> + struct ma35_pinctrl *npctl = dev_get_drvdata(dev);
> +
> + return pinctrl_force_sleep(npctl->pctl);
> +}
> +
> +int __maybe_unused ma35_pinctrl_resume(struct device *dev)
Ditto.
> +{
> + struct ma35_pinctrl *npctl = dev_get_drvdata(dev);
> +
> + return pinctrl_force_default(npctl->pctl);
> +}
...
> +struct ma35_mux_desc {
> + const char *name;
> + u32 muxval;
> +};
> +
> +struct ma35_pin_data {
> + u32 offset;
> + u32 shift;
> + struct ma35_mux_desc *muxes;
> +};
Don't pin control subsystem has already some data types suitable in this cases?
...
> +#define MA35_PIN(num, n, o, s, ...) { \
> + .number = num, \
> + .name = #n, \
Don't we have macros for this?
> + .drv_data = &(struct ma35_pin_data) { \
> + .offset = o, \
> + .shift = s, \
> + .muxes = (struct ma35_mux_desc[]) { \
> + __VA_ARGS__, { } }, \
> + }, \
> +}
...
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
Missing headers and stray ones. Please, get this into order by following IWYU
principle.
> +#include <linux/of.h>
> +#include <linux/of_device.h>
How are these being used?
> +#include <linux/pinctrl/pinctrl.h>
...
> +static const struct dev_pm_ops ma35d1_pinctrl_pm_ops = {
> + SET_LATE_SYSTEM_SLEEP_PM_OPS(ma35_pinctrl_suspend, ma35_pinctrl_resume)
> +};
New PM macros, please.
...
> +static const struct of_device_id ma35d1_pinctrl_of_match[] = {
> + { .compatible = "nuvoton,ma35d1-pinctrl", },
> + { },
No comma in the terminator entry.
> +};
...
> +static struct platform_driver ma35d1_pinctrl_driver = {
> + .probe = ma35d1_pinctrl_probe,
> + .driver = {
> + .name = "ma35d1-pinctrl",
> + .pm = &ma35d1_pinctrl_pm_ops,
New PM macro here.
> + .of_match_table = ma35d1_pinctrl_of_match,
> + },
> +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: (subset) [PATCH v12 0/7] drm/meson: add support for MIPI DSI Display
From: Jerome Brunet @ 2024-04-10 8:44 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Martin Blumenstingl, Kevin Hilman, Michael Turquette,
Stephen Boyd, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Jagan Teki, Nicolas Belin,
Neil Armstrong
Cc: devicetree, linux-kernel, linux-amlogic, linux-clk,
linux-arm-kernel, dri-devel, Conor Dooley, Lukas F. Hartmann
In-Reply-To: <20240403-amlogic-v6-4-upstream-dsi-ccf-vim3-v12-0-99ecdfdc87fc@linaro.org>
Applied to clk-meson (v6.10/drivers), thanks!
[2/7] clk: meson: add vclk driver
https://github.com/BayLibre/clk-meson/commit/bb5aa08572b5
[3/7] clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF
https://github.com/BayLibre/clk-meson/commit/b70cb1a21a54
Best regards,
--
Jerome
^ permalink raw reply
* Re: [PATCH v3 19/25] media: i2c: imx258: Change register settings for variants of the sensor
From: Sakari Ailus @ 2024-04-10 8:44 UTC (permalink / raw)
To: Dave Stevenson
Cc: Luigi311, linux-media, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel
In-Reply-To: <CAPY8ntDP-EPQK_d=5NeVM-ZTjfhtpYRq_y6PVSn9dRzxD5b1_A@mail.gmail.com>
Hi Dave,
On Fri, Apr 05, 2024 at 02:16:31PM +0100, Dave Stevenson wrote:
> Hi Sakari
>
> On Fri, 5 Apr 2024 at 11:59, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Luis, Dave,
> >
> > On Thu, Apr 04, 2024 at 04:44:05PM -0600, Luigi311 wrote:
> > > On 4/3/24 10:18, Sakari Ailus wrote:
> > > > Hi Luis, Dave,
> > > >
> > > > On Wed, Apr 03, 2024 at 09:03:48AM -0600, git@luigi311.com wrote:
> > > >> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > >>
> > > >> Sony have advised that there are variants of the IMX258 sensor which
> > > >> require slightly different register configuration to the mainline
> > > >> imx258 driver defaults.
> > > >>
> > > >> There is no available run-time detection for the variant, so add
> > > >> configuration via the DT compatible string.
> > > >>
> > > >> The Vision Components imx258 module supports PDAF, so add the
> > > >> register differences for that variant
> > > >>
> > > >> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > >> Signed-off-by: Luis Garcia <git@luigi311.com>
> > > >> ---
> > > >> drivers/media/i2c/imx258.c | 48 ++++++++++++++++++++++++++++++++++----
> > > >> 1 file changed, 44 insertions(+), 4 deletions(-)
> > > >>
> > > >> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> > > >> index 775d957c9b87..fa48da212037 100644
> > > >> --- a/drivers/media/i2c/imx258.c
> > > >> +++ b/drivers/media/i2c/imx258.c
> > > >> @@ -6,6 +6,7 @@
> > > >> #include <linux/delay.h>
> > > >> #include <linux/i2c.h>
> > > >> #include <linux/module.h>
> > > >> +#include <linux/of_device.h>
> > > >> #include <linux/pm_runtime.h>
> > > >> #include <linux/regulator/consumer.h>
> > > >> #include <media/v4l2-ctrls.h>
> > > >> @@ -321,8 +322,6 @@ static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
> > > >>
> > > >> static const struct imx258_reg mode_common_regs[] = {
> > > >> { 0x3051, 0x00 },
> > > >> - { 0x3052, 0x00 },
> > > >> - { 0x4E21, 0x14 },
> > > >> { 0x6B11, 0xCF },
> > > >> { 0x7FF0, 0x08 },
> > > >> { 0x7FF1, 0x0F },
> > > >> @@ -345,7 +344,6 @@ static const struct imx258_reg mode_common_regs[] = {
> > > >> { 0x7FA8, 0x03 },
> > > >> { 0x7FA9, 0xFE },
> > > >> { 0x7B24, 0x81 },
> > > >> - { 0x7B25, 0x00 },
> > > >> { 0x6564, 0x07 },
> > > >> { 0x6B0D, 0x41 },
> > > >> { 0x653D, 0x04 },
> > > >> @@ -460,6 +458,33 @@ static const struct imx258_reg mode_1048_780_regs[] = {
> > > >> { 0x034F, 0x0C },
> > > >> };
> > > >>
> > > >> +struct imx258_variant_cfg {
> > > >> + const struct imx258_reg *regs;
> > > >> + unsigned int num_regs;
> > > >> +};
> > > >> +
> > > >> +static const struct imx258_reg imx258_cfg_regs[] = {
> > > >> + { 0x3052, 0x00 },
> > > >> + { 0x4E21, 0x14 },
> > > >> + { 0x7B25, 0x00 },
> > > >> +};
> > > >> +
> > > >> +static const struct imx258_variant_cfg imx258_cfg = {
> > > >> + .regs = imx258_cfg_regs,
> > > >> + .num_regs = ARRAY_SIZE(imx258_cfg_regs),
> > > >> +};
> > > >> +
> > > >> +static const struct imx258_reg imx258_pdaf_cfg_regs[] = {
> > > >> + { 0x3052, 0x01 },
> > > >> + { 0x4E21, 0x10 },
> > > >> + { 0x7B25, 0x01 },
> > > >> +};
> > > >> +
> > > >> +static const struct imx258_variant_cfg imx258_pdaf_cfg = {
> > > >> + .regs = imx258_pdaf_cfg_regs,
> > > >> + .num_regs = ARRAY_SIZE(imx258_pdaf_cfg_regs),
> > > >> +};
> > > >> +
> > > >> static const char * const imx258_test_pattern_menu[] = {
> > > >> "Disabled",
> > > >> "Solid Colour",
> > > >> @@ -637,6 +662,8 @@ struct imx258 {
> > > >> struct v4l2_subdev sd;
> > > >> struct media_pad pad;
> > > >>
> > > >> + const struct imx258_variant_cfg *variant_cfg;
> > > >> +
> > > >> struct v4l2_ctrl_handler ctrl_handler;
> > > >> /* V4L2 Controls */
> > > >> struct v4l2_ctrl *link_freq;
> > > >> @@ -1104,6 +1131,14 @@ static int imx258_start_streaming(struct imx258 *imx258)
> > > >> return ret;
> > > >> }
> > > >>
> > > >> + ret = imx258_write_regs(imx258, imx258->variant_cfg->regs,
> > > >> + imx258->variant_cfg->num_regs);
> > > >> + if (ret) {
> > > >> + dev_err(&client->dev, "%s failed to set variant config\n",
> > > >> + __func__);
> > > >> + return ret;
> > > >> + }
> > > >> +
> > > >> ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP,
> > > >> IMX258_REG_VALUE_08BIT,
> > > >> imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ?
> > > >> @@ -1492,6 +1527,10 @@ static int imx258_probe(struct i2c_client *client)
> > > >>
> > > >> imx258->csi2_flags = ep.bus.mipi_csi2.flags;
> > > >>
> > > >> + imx258->variant_cfg = of_device_get_match_data(&client->dev);
> > > >
> > > > You'll also need to keep this working for ACPI based systems. I.e. in
> > > > practice remove "of_" prefix here and add the non-PDAF variant data to the
> > > > relevant ACPI ID list.
> > > >
> > >
> > > Removing of_ is easy enough and looking at all the other commits that make
> > > this change in other drivers I dont see anything else being done besides
> > > adding in the .data section that is down below for both imx258 and pdaf
> > > versions. Is that what you are referencing or is there some other place
> > > to add variant data to ACPI ID list?
> >
> > Speaking of which---are you absolutely certain there are two variants of
> > this sensor? Many sensors that have a different pixel pattern (PDAF pixels
> > or a non-Bayer pattern) can produce Bayer data when condigured so. The fact
> > that you have differing register configuration for the PDAF and non-PDAF
> > cases suggests this may well be the case.
>
> I had a discussion with our contact at Sony over the configuration,
> and Soho Enterprises who made the module I have also consulted with
> Sony (their main person is ex Sony himself).
>
> There is a spec version field in the OTP which reflects the pixel
> pattern. It has defined options of:
> - HDR pattern
> - Binning pattern
> - mono
> - non-PDAF
> - HDR HDD
>
> Sony can't release information on how to read that information from
> the sensor OTP as it is contractually locked by contracts with Intel.
> Whilst information obtained via other routes means I have checked it
> on my module as HDR pattern whilst the Nautilus platform has the
> non-PDAF variant, I'm not going to spoil our relationship with Sony by
> releasing that.
>
> It's possible that the Nautilus sensor will work happily with the
> settings required for the PDAF variant, but I have no way of testing
> that, and the registers in question are undocumented. Changing them
> blindly isn't going to make any friends, and I doubt existing platform
> users wish to rerun all their image quality tests on the sensor to
> validate the change.
>
> Unless Intel wish to release the information on reading the OTP, we
> have no way of telling the variants apart but need different register
> configurations. If there is a better way of handling that situation
> than compatible strings, then I'm open to suggestions.
>
> There's a short thread on libcamera-devel from back in 2022 where I
> was looking into this [1]
Oops! I guess we'll need these two for now at least.
We don't really have support for PDAF anyway (I'd expect this to be
documented for a driver, for instance) so I presume currently for
PDAF-variants the configuration is about "correcting" the PDAF pixels?
The problem seems to be worse on ACPI systems as there's a single HID only.
>
> Dave
>
> [1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-June/031449.html
>
> > >
> > > >> + if (!imx258->variant_cfg)
> > > >> + imx258->variant_cfg = &imx258_cfg;
> > > >> +
> > > >> /* Initialize subdev */
> > > >> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
> > > >>
> > > >> @@ -1579,7 +1618,8 @@ MODULE_DEVICE_TABLE(acpi, imx258_acpi_ids);
> > > >> #endif
> > > >>
> > > >> static const struct of_device_id imx258_dt_ids[] = {
> > > >> - { .compatible = "sony,imx258" },
> > > >> + { .compatible = "sony,imx258", .data = &imx258_cfg },
> > > >> + { .compatible = "sony,imx258-pdaf", .data = &imx258_pdaf_cfg },
> > > >> { /* sentinel */ }
> > > >> };
> > > >> MODULE_DEVICE_TABLE(of, imx258_dt_ids);
> > > >
> > >
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* [PATCH v2] arm64: dts: mediatek: mt8192: Add missing trip point in thermal zone
From: Hsin-Te Yuan @ 2024-04-10 8:40 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nícolas F. R. A. Prado,
Bernhard Rosenkränzer, Balsam CHIHI, Alexandre Mergnat
Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
Hsin-Te Yuan
According to Documentation/driver-api/thermal/power_allocator.rst, there
should be two passive trip points. Adding the missing trip point to
ensure that the governor works optimally.
Fixes: c7a728051f4e ("arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones")
Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org>
---
Changes in v2:
- Clearify the reason of adding another passive trip point
- Link to v1: https://lore.kernel.org/r/20240410-upstream-torvalds-master-v1-1-852e903f0cec@chromium.org
---
arch/arm64/boot/dts/mediatek/mt8192.dtsi | 40 ++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index 05e401670bced..08d8bccc84669 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -1959,6 +1959,11 @@ cpu0-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU0>;
trips {
+ cpu0_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu0_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
@@ -1989,6 +1994,11 @@ cpu1-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU1>;
trips {
+ cpu1_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu1_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
@@ -2019,6 +2029,11 @@ cpu2-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU2>;
trips {
+ cpu2_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu2_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
@@ -2049,6 +2064,11 @@ cpu3-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU3>;
trips {
+ cpu3_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu3_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
@@ -2079,6 +2099,11 @@ cpu4-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU0>;
trips {
+ cpu4_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu4_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
@@ -2109,6 +2134,11 @@ cpu5-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU1>;
trips {
+ cpu5_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu5_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
@@ -2139,6 +2169,11 @@ cpu6-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU2>;
trips {
+ cpu6_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu6_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
@@ -2169,6 +2204,11 @@ cpu7-thermal {
thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU3>;
trips {
+ cpu7_thres: trip-point {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
cpu7_alert: trip-alert {
temperature = <85000>;
hysteresis = <2000>;
---
base-commit: 20cb38a7af88dc40095da7c2c9094da3873fea23
change-id: 20240410-upstream-torvalds-master-40aeff5416c7
Best regards,
--
Hsin-Te Yuan <yuanhsinte@chromium.org>
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: mediatek: mt8183: Refactor thermal zones
From: AngeloGioacchino Del Regno @ 2024-04-10 8:30 UTC (permalink / raw)
To: linux-mediatek
Cc: robh, krzysztof.kozlowski+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, bchihi, bero, amergnat, nfraprado,
michael.kao, mka, devicetree, linux-kernel, linux-arm-kernel,
kernel
In-Reply-To: <20240410083002.1357857-1-angelogioacchino.delregno@collabora.com>
The thermal zones in MT8183 had cryptic names and all of them, apart
from the cpu-thermal zone, had no thermal trips, hence those were not
probed at all.
Refactor the tzts1..5 and tztsABB thermal zones to add the correct
thermal trips and give them a meaningful name, corresponding to the
actually monitored thermal zone.
While at it, also rename the thermal sensor node to "thermal-sensor".
Now the thermal zones are probing and their temperatures can be read.
Fixes: b325ce39785b ("arm64: dts: mt8183: add thermal zone node")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 136 +++++++++++++++++------
1 file changed, 102 insertions(+), 34 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 93dfbf130231..1978156cae76 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1183,7 +1183,7 @@ spi0: spi@1100a000 {
status = "disabled";
};
- thermal: thermal@1100b000 {
+ thermal: thermal-sensor@1100b000 {
#thermal-sensor-cells = <1>;
compatible = "mediatek,mt8183-thermal";
reg = <0 0x1100b000 0 0xc00>;
@@ -2089,61 +2089,129 @@ THERMAL_NO_LIMIT
};
};
- /* The tzts1 ~ tzts6 don't need to polling */
- /* The tzts1 ~ tzts6 don't need to thermal throttle */
-
- tzts1: tzts1 {
- polling-delay-passive = <0>;
- polling-delay = <0>;
+ tzts1: soc-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
thermal-sensors = <&thermal 1>;
sustainable-power = <5000>;
- trips {};
- cooling-maps {};
+ trips {
+ soc_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ soc_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
};
- tzts2: tzts2 {
- polling-delay-passive = <0>;
- polling-delay = <0>;
+ tzts2: gpu-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
thermal-sensors = <&thermal 2>;
sustainable-power = <5000>;
- trips {};
- cooling-maps {};
+
+ trips {
+ gpu_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
};
- tzts3: tzts3 {
- polling-delay-passive = <0>;
- polling-delay = <0>;
+ tzts3: md1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
thermal-sensors = <&thermal 3>;
sustainable-power = <5000>;
- trips {};
- cooling-maps {};
+
+ trips {
+ md1_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ md1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
};
- tzts4: tzts4 {
- polling-delay-passive = <0>;
- polling-delay = <0>;
+ tzts4: cpu-little-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
thermal-sensors = <&thermal 4>;
sustainable-power = <5000>;
- trips {};
- cooling-maps {};
+
+ trips {
+ cpul_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpul_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
};
- tzts5: tzts5 {
- polling-delay-passive = <0>;
- polling-delay = <0>;
+ tzts5: cpu-big-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
thermal-sensors = <&thermal 5>;
sustainable-power = <5000>;
- trips {};
- cooling-maps {};
+
+ trips {
+ cpub_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpub_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
};
- tztsABB: tztsABB {
- polling-delay-passive = <0>;
- polling-delay = <0>;
+ tztsABB: tsabb-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
thermal-sensors = <&thermal 6>;
sustainable-power = <5000>;
- trips {};
- cooling-maps {};
+
+ trips {
+ tsabb_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ tsabb_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
};
};
};
--
2.44.0
^ permalink raw reply related
* [PATCH 2/3] arm64: dts: mediatek: mt8192: Fix GPU thermal zone name for SVS
From: AngeloGioacchino Del Regno @ 2024-04-10 8:30 UTC (permalink / raw)
To: linux-mediatek
Cc: robh, krzysztof.kozlowski+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, bchihi, bero, amergnat, nfraprado,
michael.kao, mka, devicetree, linux-kernel, linux-arm-kernel,
kernel
In-Reply-To: <20240410083002.1357857-1-angelogioacchino.delregno@collabora.com>
This SoC has two GPU related thermal zones: the primary zone must be
called "gpu-thermal" for SVS to pick it up.
Fixes: c7a728051f4e ("arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
arch/arm64/boot/dts/mediatek/mt8192.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index f3b0da2399a0..1f0f076272a3 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -2236,7 +2236,7 @@ vpu1_crit: trip-crit {
};
};
- gpu0-thermal {
+ gpu-thermal {
polling-delay = <1000>;
polling-delay-passive = <250>;
thermal-sensors = <&lvts_ap MT8192_AP_GPU0>;
--
2.44.0
^ permalink raw reply related
* [PATCH 1/3] arm64: dts: mediatek: mt8195: Fix GPU thermal zone name for SVS
From: AngeloGioacchino Del Regno @ 2024-04-10 8:30 UTC (permalink / raw)
To: linux-mediatek
Cc: robh, krzysztof.kozlowski+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, bchihi, bero, amergnat, nfraprado,
michael.kao, mka, devicetree, linux-kernel, linux-arm-kernel,
kernel
In-Reply-To: <20240410083002.1357857-1-angelogioacchino.delregno@collabora.com>
This SoC has two GPU related thermal zones: the primary zone must be
called "gpu-thermal" for SVS to pick it up.
Fixes: 1e5b6725199f ("arm64: dts: mediatek: mt8195: Add AP domain thermal zones")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 30ee39045e7b..d3254ff74953 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -3877,7 +3877,7 @@ vpu1_crit: trip-crit {
};
};
- gpu0-thermal {
+ gpu-thermal {
polling-delay = <1000>;
polling-delay-passive = <250>;
thermal-sensors = <&lvts_ap MT8195_AP_GPU0>;
--
2.44.0
^ permalink raw reply related
* [PATCH 0/3] Address thermal zone issues on MT8183/95/92
From: AngeloGioacchino Del Regno @ 2024-04-10 8:29 UTC (permalink / raw)
To: linux-mediatek
Cc: robh, krzysztof.kozlowski+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, bchihi, bero, amergnat, nfraprado,
michael.kao, mka, devicetree, linux-kernel, linux-arm-kernel,
kernel
Thermal zones on some MediaTek device trees either have cryptic
names or wrong ones.
Having the right names is important for both human readability and
for MediaTek SVS functionality: fix those.
AngeloGioacchino Del Regno (3):
arm64: dts: mediatek: mt8195: Fix GPU thermal zone name for SVS
arm64: dts: mediatek: mt8192: Fix GPU thermal zone name for SVS
arm64: dts: mediatek: mt8183: Refactor thermal zones
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 136 +++++++++++++++++------
arch/arm64/boot/dts/mediatek/mt8192.dtsi | 2 +-
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
3 files changed, 104 insertions(+), 36 deletions(-)
--
2.44.0
^ permalink raw reply
* Re: [PATCH] arm64: dts: mediatek: mt8192: Add missing trip point in thermal zone
From: Hsin-Te Yuan @ 2024-04-10 8:27 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: Hsin-Te Yuan, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Nícolas F. R. A. Prado,
Bernhard Rosenkränzer, Balsam CHIHI, Alexandre Mergnat,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <c3d05b3e-f784-4606-9634-52dc6feee318@collabora.com>
Hi Angelo,
According to the document
(https://docs.kernel.org/driver-api/thermal/power_allocator.html),
there should be two passive trip points. It seems that the switch-on
temperature will be 0 if we only have one trip point, which hurts
performance. I'll send v2 to explain this more clearly.
Regards,
Hsin-Te
On Wed, Apr 10, 2024 at 3:43 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 10/04/24 08:20, Hsin-Te Yuan ha scritto:
> > Add the missing passive trip point which is expected by kernel.
> >
> > Fixes: c7a728051f4e ("arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones")
> > Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org>
>
> Sorry, I don't understand what you're trying to solve here.
>
> All of the thermal zones in mt8192.dtsi already do have a passive trip point
> which is also used in cooling-maps.
>
> Can you please describe the issue?
>
> Thanks,
> Angelo
>
> > ---
> > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 40 ++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > index 05e401670bced..08d8bccc84669 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > @@ -1959,6 +1959,11 @@ cpu0-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU0>;
> >
> > trips {
> > + cpu0_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu0_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > @@ -1989,6 +1994,11 @@ cpu1-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU1>;
> >
> > trips {
> > + cpu1_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu1_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > @@ -2019,6 +2029,11 @@ cpu2-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU2>;
> >
> > trips {
> > + cpu2_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu2_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > @@ -2049,6 +2064,11 @@ cpu3-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU3>;
> >
> > trips {
> > + cpu3_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu3_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > @@ -2079,6 +2099,11 @@ cpu4-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU0>;
> >
> > trips {
> > + cpu4_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu4_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > @@ -2109,6 +2134,11 @@ cpu5-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU1>;
> >
> > trips {
> > + cpu5_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu5_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > @@ -2139,6 +2169,11 @@ cpu6-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU2>;
> >
> > trips {
> > + cpu6_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu6_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> > @@ -2169,6 +2204,11 @@ cpu7-thermal {
> > thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU3>;
> >
> > trips {
> > + cpu7_thres: trip-point {
> > + temperature = <68000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > cpu7_alert: trip-alert {
> > temperature = <85000>;
> > hysteresis = <2000>;
> >
> > ---
> > base-commit: 20cb38a7af88dc40095da7c2c9094da3873fea23
> > change-id: 20240410-upstream-torvalds-master-40aeff5416c7
> >
> > Best regards,
>
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: sm8650: add description of CCI controllers
From: Krzysztof Kozlowski @ 2024-04-10 8:26 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bjorn Andersson, Konrad Dybcio,
Jagadeesh Kona
Cc: Krzysztof Kozlowski, Neil Armstrong, linux-arm-msm, devicetree
In-Reply-To: <20240410074951.447898-1-vladimir.zapolskiy@linaro.org>
On 10/04/2024 09:49, Vladimir Zapolskiy wrote:
> Qualcomm SM8650 SoC has three CCI controllers with two I2C busses
> connected to each of them.
>
> The CCI controllers on SM8650 are compatible with the ones found on
> many other older generations of Qualcomm SoCs.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> The change is based and depends on a patch series from Jagadeesh Kona:
>
> https://lore.kernel.org/linux-arm-msm/20240321092529.13362-1-quic_jkona@quicinc.com/
>
> It might be an option to add this change right to the series,
> since it anyway requires a respin.
>
> A new compatible value "qcom,sm8650-cci" is NOT added to
> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml , because
> the controller IP description and selection is covered by a generic
> compatible value "qcom,msm8996-cci".
I do not understand this reasoning. So you introduce known errors
because errors are ok?
How does it pass dtbs_check validation?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 3/4] dt-bindings: display: panel: Add compatible for IVO t109nw41
From: Krzysztof Kozlowski @ 2024-04-10 8:24 UTC (permalink / raw)
To: Cong Yang, sam, neil.armstrong, daniel, dianders, airlied,
robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20240410071439.2152588-4-yangcong5@huaqin.corp-partner.google.com>
On 10/04/2024 09:14, Cong Yang wrote:
> The IVO t109nw41 is a 11.0" WUXGA TFT LCD panel, which fits in nicely with
> the existing panel-boe-tv101wum-nl6 driver. Hence, we add a new compatible
> with panel specific config.
>
> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> ---
> .../devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> index 50351dd3d6e5..f15588a2641c 100644
> --- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> @@ -38,6 +38,8 @@ properties:
> - starry,ili9882t
> # Boe nv110wum-l60 11.0" WUXGA TFT LCD panel
> - boe,nv110wum-l60
> + # Ivo t109nw41 11.0" WUXGA TFT LCD panel
> + - ivo,t109nw41
>
Same problem...
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 1/4] dt-bindings: display: panel: Add compatible for BOE nv110wum-l60
From: Krzysztof Kozlowski @ 2024-04-10 8:23 UTC (permalink / raw)
To: Cong Yang, sam, neil.armstrong, daniel, dianders, airlied,
robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20240410071439.2152588-2-yangcong5@huaqin.corp-partner.google.com>
On 10/04/2024 09:14, Cong Yang wrote:
> The BOE nv110wum-l60 is a 11.0" WUXGA TFT LCD panel, which fits in nicely
> with the existing panel-boe-tv101wum-nl6 driver. Hence, we add a new
> compatible with panel specific config.
>
> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> ---
> .../devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> index 906ef62709b8..50351dd3d6e5 100644
> --- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> @@ -36,6 +36,8 @@ properties:
> - starry,himax83102-j02
> # STARRY ili9882t 10.51" WUXGA TFT LCD panel
> - starry,ili9882t
> + # Boe nv110wum-l60 11.0" WUXGA TFT LCD panel
> + - boe,nv110wum-l60
Isn't the list ordered?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
From: Neeraj Sanjay Kale @ 2024-04-10 8:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, marcel@holtmann.org, luiz.dentz@gmail.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org
Cc: linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Amitkumar Karwar, Rohit Fule, Sherry Sun, Luke Wang, Bough Chen,
LnxRevLi
In-Reply-To: <e9cef5dd-c4fc-43c0-839b-d311d87a28ca@linaro.org>
Hi Krzysztof,
Thank you for the review. I have made the changes you requested and sent out the v3 patch.
Thanks,
Neeraj
>
> On 08/04/2024 15:22, Neeraj Sanjay Kale wrote:
> > This adds a new optional device tree property called firware-name.
> >
> > Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
>
> BTW, there is no such device as btnxpuart. Bindings are for hardware.
> With corrected subject:
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> > ---
> > v2: Add maxItems, simplify description, remove "nxp/". (Krzysztof)
> > ---
> > .../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> > b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> > index f01a3988538c..6774cc4d6a9e 100644
> > ---
> > a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> > +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.y
> > +++ aml
> > @@ -31,6 +31,11 @@ properties:
> > This property depends on the module vendor's
> > configuration.
> >
> > + firmware-name:
> > + maxItems: 1
> > + description:
> > + Specify firmware file name.
>
> Drop description, redundant. You did not say anything different than property
> is saying already.
^ permalink raw reply
* [PATCH v3 2/2] Bluetooth: btnxpuart: Update firmware names
From: Neeraj Sanjay Kale @ 2024-04-10 8:10 UTC (permalink / raw)
To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
ziniu.wang_1, haibo.chen, LnxRevLi
In-Reply-To: <20240410081049.775382-1-neeraj.sanjaykale@nxp.com>
This updates the firmware names of 3 chipsets: w8987, w8997, w9098.
These changes are been done to standardize chip specific firmware
file names.
To allow user to use older firmware file names, a new device tree
property has been introduced called firmware-name, which will override
the hardcoded firmware names in the driver.
Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
v2: Remove "nxp/" from all firmware name definitions to be inline with
firware file name read from device tree file. (Krzysztof)
---
drivers/bluetooth/btnxpuart.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 0b93c2ff29e4..4442d911eba8 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -33,16 +33,16 @@
/* NXP HW err codes */
#define BTNXPUART_IR_HW_ERR 0xb0
-#define FIRMWARE_W8987 "nxp/uartuart8987_bt.bin"
-#define FIRMWARE_W8997 "nxp/uartuart8997_bt_v4.bin"
-#define FIRMWARE_W9098 "nxp/uartuart9098_bt_v1.bin"
-#define FIRMWARE_IW416 "nxp/uartiw416_bt_v0.bin"
-#define FIRMWARE_IW612 "nxp/uartspi_n61x_v1.bin.se"
-#define FIRMWARE_IW624 "nxp/uartiw624_bt.bin"
-#define FIRMWARE_SECURE_IW624 "nxp/uartiw624_bt.bin.se"
-#define FIRMWARE_AW693 "nxp/uartaw693_bt.bin"
-#define FIRMWARE_SECURE_AW693 "nxp/uartaw693_bt.bin.se"
-#define FIRMWARE_HELPER "nxp/helper_uart_3000000.bin"
+#define FIRMWARE_W8987 "uart8987_bt_v0.bin"
+#define FIRMWARE_W8997 "uart8997_bt_v4.bin"
+#define FIRMWARE_W9098 "uart9098_bt_v1.bin"
+#define FIRMWARE_IW416 "uartiw416_bt_v0.bin"
+#define FIRMWARE_IW612 "uartspi_n61x_v1.bin.se"
+#define FIRMWARE_IW624 "uartiw624_bt.bin"
+#define FIRMWARE_SECURE_IW624 "uartiw624_bt.bin.se"
+#define FIRMWARE_AW693 "uartaw693_bt.bin"
+#define FIRMWARE_SECURE_AW693 "uartaw693_bt.bin.se"
+#define FIRMWARE_HELPER "helper_uart_3000000.bin"
#define CHIP_ID_W9098 0x5c03
#define CHIP_ID_IW416 0x7201
@@ -685,13 +685,19 @@ static bool process_boot_signature(struct btnxpuart_dev *nxpdev)
static int nxp_request_firmware(struct hci_dev *hdev, const char *fw_name)
{
struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
+ const char *fw_name_dt;
int err = 0;
if (!fw_name)
return -ENOENT;
if (!strlen(nxpdev->fw_name)) {
- snprintf(nxpdev->fw_name, MAX_FW_FILE_NAME_LEN, "%s", fw_name);
+ if (strcmp(fw_name, FIRMWARE_HELPER) &&
+ !device_property_read_string(&nxpdev->serdev->dev,
+ "firmware-name",
+ &fw_name_dt))
+ fw_name = fw_name_dt;
+ snprintf(nxpdev->fw_name, MAX_FW_FILE_NAME_LEN, "nxp/%s", fw_name);
bt_dev_dbg(hdev, "Request Firmware: %s", nxpdev->fw_name);
err = request_firmware(&nxpdev->fw, nxpdev->fw_name, &hdev->dev);
--
2.34.1
^ permalink raw reply related
* [PATCH v3 1/2] dt-bindings: net: bluetooth: nxp: Add firmware-name property
From: Neeraj Sanjay Kale @ 2024-04-10 8:10 UTC (permalink / raw)
To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
ziniu.wang_1, haibo.chen, LnxRevLi
In-Reply-To: <20240410081049.775382-1-neeraj.sanjaykale@nxp.com>
This adds a new optional device tree property called firware-name.
Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
v2: Add maxItems, simplify description, remove "nxp/". (Krzysztof)
v3: Corrected subject. Dropped description. (Krzysztof)
---
.../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
index f01a3988538c..37a65badb448 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
@@ -31,6 +31,9 @@ properties:
This property depends on the module vendor's
configuration.
+ firmware-name:
+ maxItems: 1
+
required:
- compatible
@@ -42,5 +45,6 @@ examples:
bluetooth {
compatible = "nxp,88w8987-bt";
fw-init-baudrate = <3000000>;
+ firmware-name = "uartuart8987_bt_v0.bin";
};
};
--
2.34.1
^ permalink raw reply related
* [PATCH v3 0/2] Bluetooth: btnxpuart: Update firmware names
From: Neeraj Sanjay Kale @ 2024-04-10 8:10 UTC (permalink / raw)
To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
ziniu.wang_1, haibo.chen, LnxRevLi
This patch series updates the BT firmware file names in BTNXPUART
driver, and adds a new optional firmware-name device tree property to
override the firmware file names hardcoded in the driver. This will
allow user to continue using the older firmware files.
This change is necessary as newer firmware releases will have
standardized naming convention aligned across all newer and legacy
chipsets.
Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
Neeraj Sanjay Kale (2):
dt-bindings: net: bluetooth: nxp: Add firmware-name property
Bluetooth: btnxpuart: Update firmware names
.../net/bluetooth/nxp,88w8987-bt.yaml | 4 +++
drivers/bluetooth/btnxpuart.c | 28 +++++++++++--------
2 files changed, 21 insertions(+), 11 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH net-next v7 13/17] net: pse-pd: Use regulator framework within PSE framework
From: Kory Maincent @ 2024-04-10 8:09 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Jonathan Corbet,
Luis Chamberlain, Russ Weight, Greg Kroah-Hartman,
Rafael J. Wysocki, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Oleksij Rempel, Mark Brown, Frank Rowand, Andrew Lunn,
Heiner Kallweit, Russell King, Thomas Petazzoni, netdev,
linux-kernel, linux-doc, devicetree, Dent Project, kernel,
Maxime Chevallier
In-Reply-To: <20240409141621.392bd34b@kernel.org>
On Tue, 9 Apr 2024 14:16:21 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 09 Apr 2024 17:04:03 +0200 Kory Maincent wrote:
> > -static inline struct pse_control *of_pse_control_get(struct device_node
> > *node) +static inline struct pse_control *of_pse_control_get(struct device
> > *dev,
> > + struct device_node
> > *node)
>
> One of the related patches breaks the build:
>
> drivers/net/mdio/fwnode_mdio.c: In function ‘fwnode_find_pse_control’:
> drivers/net/mdio/fwnode_mdio.c:32:35: error: passing argument 1 of
> ‘of_pse_control_get’ from incompatible pointer type
> [-Werror=incompatible-pointer-types] 32 | psec =
> of_pse_control_get(np); | ^~ |
> | | struct device_node
> * In file included from drivers/net/mdio/fwnode_mdio.c:13:
> ./include/linux/pse-pd/pse.h:157:69: note: expected ‘struct device *’ but
> argument is of type ‘struct device_node *’ 157 | static inline struct
> pse_control *of_pse_control_get(struct device *dev, |
> ~~~~~~~~~~~~~~~^~~
> drivers/net/mdio/fwnode_mdio.c:32:16: error: too few arguments to function
> ‘of_pse_control_get’ 32 | psec = of_pse_control_get(np); |
> ^~~~~~~~~~~~~~~~~~ In file included from
> drivers/net/mdio/fwnode_mdio.c:13: ./include/linux/pse-pd/pse.h:157:35: note:
> declared here 157 | static inline struct pse_control
> *of_pse_control_get(struct device *dev, |
> ^~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors
Arg, a leftover of an old version of the series.
Thanks for the report.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v4 1/2] clk: starfive: jh7110-sys: Add notifier for PLL clock
From: Stephen Boyd @ 2024-04-10 7:58 UTC (permalink / raw)
To: Conor Dooley, Emil Renner Berthing, Krzysztof Kozlowski,
Michael Turquette, Rob Herring, Xingyu Wu
Cc: Emil Renner Berthing, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Hal Feng, Xingyu Wu, linux-kernel, linux-clk, linux-riscv,
devicetree
In-Reply-To: <20240410033148.213991-2-xingyu.wu@starfivetech.com>
Quoting Xingyu Wu (2024-04-09 20:31:47)
> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-sys.c b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> index 8f5e5abfa178..adf62e4d94e4 100644
> --- a/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> +++ b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> @@ -385,6 +385,32 @@ int jh7110_reset_controller_register(struct jh71x0_clk_priv *priv,
> }
> EXPORT_SYMBOL_GPL(jh7110_reset_controller_register);
>
> +/*
> + * This clock notifier is called when the rate of PLL0 clock is to be change,
s/change,/changed./
> + * The cpu_root clock should save curent parent clock and swicth its parent
s/swicth/switch/
> + * clock to osc before PLL0 rate will be changed. And switch its parent clock
> + * back after PLL rate finished.
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: mfd: Add ROHM BD71879
From: Matti Vaittinen @ 2024-04-10 7:57 UTC (permalink / raw)
To: Andreas Kemnade, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
devicetree, linux-kernel
In-Reply-To: <20240404195423.666446-2-andreas@kemnade.info>
On 4/4/24 22:54, Andreas Kemnade wrote:
> As this chip was seen in several devices in the wild, add it.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> .../devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> index 0b62f854bf6b..07f99738fcf6 100644
> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> @@ -17,7 +17,12 @@ description: |
>
> properties:
> compatible:
> - const: rohm,bd71828
> + oneOf:
> + - const: rohm,bd71828
> +
> + - items:
> + - const: rohm,bd71879
> + - const: rohm,bd71828
>
> reg:
> description:
Am I correct, this reads as:
Either
compatible = rohm,bd71828
or
compatible = rohm,bd71879, rohm,bd71828
but not compatible = rohm,bd71879?
If so, this looks ok to me. :)
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: riscv: add Milk-V Duo S board compatibles
From: Michael Opdenacker @ 2024-04-10 7:55 UTC (permalink / raw)
To: Inochi Amaoto, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Chen Wang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Chao Wei
Cc: michael.opdenacker, linux-riscv, devicetree, linux-kernel
In-Reply-To: <IA1PR20MB49532EC9B654B5B1C2538851BB062@IA1PR20MB4953.namprd20.prod.outlook.com>
Hi Inochi
Thanks for the reviews!
On 4/10/24 at 09:19, Inochi Amaoto wrote:
> On Wed, Apr 10, 2024 at 08:22:53AM +0200, michael.opdenacker@bootlin.com wrote:
>> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>>
>> Document the compatible strings for the Milk-V Duo S board[1] which uses
>> the SOPHGO SG2000 SoC, compatible with the SOPHGO CV1800B SoC[2].
>>
>> Link: https://milkv.io/duo-s [1]
>> Link: https://en.sophgo.com/product/introduce/cv180xB.html [2]
>>
>> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
>> ---
>> Documentation/devicetree/bindings/riscv/sophgo.yaml | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/riscv/sophgo.yaml b/Documentation/devicetree/bindings/riscv/sophgo.yaml
>> index 9bc813dad098..1837bc550056 100644
>> --- a/Documentation/devicetree/bindings/riscv/sophgo.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/sophgo.yaml
>> @@ -21,6 +21,7 @@ properties:
>> - items:
>> - enum:
>> - milkv,duo
>> + - milkv,duos
>> - const: sophgo,cv1800b
> Why not adding sg2000 property? They are different series.
> IIRC, it at least a cv1813h not cv1800b. I suggest checking
> the vendor SDK to get the right board compatibles.
Here's what there was in the vendor provided DTB, if I noted it correctly:
compatible = "cvitek,cv181x";
Cheers
Michael
--
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
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