* Re: [PATCH 3/3] of: Use scope based of_node_put() cleanups
From: Rob Herring @ 2024-04-05 13:00 UTC (permalink / raw)
To: Saravana Kannan; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <CAGETcx8Wd5OsHWiGSASWkQQtof0D-ScwYsvq9hWizV3DFC27gA@mail.gmail.com>
On Thu, Apr 4, 2024 at 6:22 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
> >
> > Use the relatively new scope based of_node_put() cleanup to simplify
> > function exit handling. Doing so reduces the chances of forgetting an
> > of_node_put() and simplifies error paths by avoiding the need for goto
> > statements.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > drivers/of/address.c | 60 ++++++++++++++++-----------------------------------
> > drivers/of/property.c | 22 ++++++-------------
> > 2 files changed, 26 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index ae46a3605904..f7b2d535a6d1 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -491,7 +491,6 @@ static u64 __of_translate_address(struct device_node *dev,
> > const __be32 *in_addr, const char *rprop,
> > struct device_node **host)
> > {
> > - struct device_node *parent = NULL;
> > struct of_bus *bus, *pbus;
> > __be32 addr[OF_MAX_ADDR_CELLS];
> > int na, ns, pna, pns;
> > @@ -504,7 +503,7 @@ static u64 __of_translate_address(struct device_node *dev,
> >
> > *host = NULL;
> > /* Get parent & match bus type */
> > - parent = get_parent(dev);
> > + struct device_node *parent __free(device_node) = get_parent(dev);
>
> Can we leave the variable definition where it was? We generally define
> all the variables up top. So, defining the one variable in the middle
> feels weird. I at least get when we do this inside for/if blocks. But
> randomly in the middle feels weird.
There's an 'of_node_get(dev);' before this. Ordering wise, we need to
hold the ref on the child before we get its parent. I suppose I can
also convert that to use the cleanups. I'll have to add another local
ptr to do that though.
>
> Similar comments in other places. Since both kfree() and of_put() can
> both handle NULL pointers, I'd be surprised if we HAVE to combine
> these lines.
https://lore.kernel.org/all/CAHk-=wgRHiV5VSxtfXA4S6aLUmcQYEuB67u3BJPJPtuESs1JyA@mail.gmail.com/
^ permalink raw reply
* Re: [RESEND v7 14/37] clk: Compatible with narrow registers
From: Geert Uytterhoeven @ 2024-04-05 12:56 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <9c1d56d37f5d3780d3c506ae680133b6bdaa5fdc.1712207606.git.ysato@users.sourceforge.jp>
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> divider and gate only support 32-bit registers.
> Older hardware uses narrower registers, so I want to be able to handle
> 8-bit and 16-bit wide registers.
>
> Seven clk_divider flags are used, and if I add flags for 8bit access and
> 16bit access, 8bit will not be enough, so I expanded it to u16.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Thanks for the update!
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -26,20 +26,38 @@
> * parent - fixed parent. No clk_set_parent support
> */
>
> -static inline u32 clk_div_readl(struct clk_divider *divider)
> -{
> - if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> - return ioread32be(divider->reg);
> -
> - return readl(divider->reg);
> +static inline u32 clk_div_read(struct clk_divider *divider)
> +{
> + if (divider->flags & CLK_DIVIDER_REG_8BIT)
When you need curly braces in one branch of an if/else statement,
please use curly braces in all branches (everywhere).
> + return readb(divider->reg);
> + else if (divider->flags & CLK_DIVIDER_REG_16BIT) {
> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> + return ioread16be(divider->reg);
> + else
> + return readw(divider->reg);
> + } else {
> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> + return ioread32be(divider->reg);
> + else
> + return readl(divider->reg);
> + }
> }
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -137,12 +155,30 @@ struct clk_hw *__clk_hw_register_gate(struct device *dev,
> struct clk_init_data init = {};
> int ret = -EINVAL;
>
> + /* validate register size option and bit_idx */
> if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
> if (bit_idx > 15) {
> pr_err("gate bit exceeds LOWORD field\n");
> return ERR_PTR(-EINVAL);
> }
> }
> + if (clk_gate_flags & CLK_GATE_REG_16BIT) {
> + if (bit_idx > 15) {
> + pr_err("gate bit exceeds 16 bits\n");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> + if (clk_gate_flags & CLK_GATE_REG_8BIT) {
> + if (bit_idx > 7) {
> + pr_err("gate bit exceeds 8 bits\n");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> + if ((clk_gate_flags & CLK_GATE_HIWORD_MASK) &&
If you use parentheses around "a & b" here...
> + clk_gate_flags & (CLK_GATE_REG_8BIT | CLK_GATE_REG_16BIT)) {
please add parentheses here, too.
> + pr_err("HIWORD_MASK required 32-bit register\n");
> + return ERR_PTR(-EINVAL);
> + }
>
> /* allocate the gate */
> gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 4a537260f655..eaa6ff1d0b2e 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np);
> * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used for
> * the gate register. Setting this flag makes the register accesses big
> * endian.
> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses 8bit.
> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses 16bit.
> */
> struct clk_gate {
> struct clk_hw hw;
> void __iomem *reg;
> u8 bit_idx;
> - u8 flags;
> + u32 flags;
(from my comments on v6)
There is no need to increase the size of the flags field for the gate clock.
> spinlock_t *lock;
> };
>
> @@ -675,13 +681,17 @@ struct clk_div_table {
> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
> * for the divider register. Setting this flag makes the register accesses
> * big endian.
> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses 8bit.
> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses 16bit.
> */
> struct clk_divider {
> struct clk_hw hw;
> void __iomem *reg;
> u8 shift;
> u8 width;
> - u8 flags;
> + u16 flags;
> const struct clk_div_table *table;
> spinlock_t *lock;
> };
> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
> struct device_node *np, const char *name,
> const char *parent_name, const struct clk_hw *parent_hw,
> const struct clk_parent_data *parent_data, unsigned long flags,
> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
"u16 clk_divider_flags", to match clk_divider.flags.
> const struct clk_div_table *table, spinlock_t *lock);
> struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
> struct device_node *np, const char *name,
> const char *parent_name, const struct clk_hw *parent_hw,
> const struct clk_parent_data *parent_data, unsigned long flags,
> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
Likewise.
> const struct clk_div_table *table, spinlock_t *lock);
> struct clk *clk_register_divider_table(struct device *dev, const char *name,
> const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 shift, u8 width,
> - u8 clk_divider_flags, const struct clk_div_table *table,
> + u32 clk_divider_flags, const struct clk_div_table *table,
Likewise.
> spinlock_t *lock);
> /**
> * clk_register_divider - register a divider clock with the clock framework
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH 2/2] arm64: dts: mediatek: mt7988: add I2C controllers
From: Rafał Miłecki @ 2024-04-05 12:55 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
Rafał Miłecki
In-Reply-To: <20240405125549.11972-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
MT7988 has three on-SoC I2C controllers that are the same hardware
blocks as already noticed on MT7981 chipsets.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 45 +++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
index 27098f724b7a..b4dc81881cc7 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
@@ -124,6 +124,51 @@ pwm@10048000 {
status = "disabled";
};
+ i2c@11003000 {
+ compatible = "mediatek,mt7981-i2c";
+ reg = <0 0x11003000 0 0x1000>,
+ <0 0x10217080 0 0x80>;
+ interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&infracfg CLK_INFRA_I2C_BCK>,
+ <&infracfg CLK_INFRA_66M_AP_DMA_BCK>;
+ clock-names = "main", "dma";
+ clock-div = <1>;
+ clock-frequency = <100000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c@11004000 {
+ compatible = "mediatek,mt7981-i2c";
+ reg = <0 0x11004000 0 0x1000>,
+ <0 0x10217100 0 0x80>;
+ interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&infracfg CLK_INFRA_I2C_BCK>,
+ <&infracfg CLK_INFRA_66M_AP_DMA_BCK>;
+ clock-names = "main", "dma";
+ clock-div = <1>;
+ clock-frequency = <100000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c@11005000 {
+ compatible = "mediatek,mt7981-i2c";
+ reg = <0 0x11005000 0 0x1000>,
+ <0 0x10217180 0 0x80>;
+ interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&infracfg CLK_INFRA_I2C_BCK>,
+ <&infracfg CLK_INFRA_66M_AP_DMA_BCK>;
+ clock-names = "main", "dma";
+ clock-div = <1>;
+ clock-frequency = <100000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
usb@11190000 {
compatible = "mediatek,mt7988-xhci", "mediatek,mtk-xhci";
reg = <0 0x11190000 0 0x2e00>,
--
2.35.3
^ permalink raw reply related
* [PATCH 1/2] arm64: dts: mediatek: mt7988: add PWM controller
From: Rafał Miłecki @ 2024-04-05 12:55 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
MT7988 has on-SoC controller that can control up to 8 PWM interfaces.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
index 3eb5396dea22..27098f724b7a 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
@@ -105,6 +105,25 @@ clock-controller@1001e000 {
#clock-cells = <1>;
};
+ pwm@10048000 {
+ compatible = "mediatek,mt7988-pwm";
+ reg = <0 0x10048000 0 0x1000>;
+ clocks = <&infracfg CLK_INFRA_66M_PWM_BCK>,
+ <&infracfg CLK_INFRA_66M_PWM_HCK>,
+ <&infracfg CLK_INFRA_66M_PWM_CK1>,
+ <&infracfg CLK_INFRA_66M_PWM_CK2>,
+ <&infracfg CLK_INFRA_66M_PWM_CK3>,
+ <&infracfg CLK_INFRA_66M_PWM_CK4>,
+ <&infracfg CLK_INFRA_66M_PWM_CK5>,
+ <&infracfg CLK_INFRA_66M_PWM_CK6>,
+ <&infracfg CLK_INFRA_66M_PWM_CK7>,
+ <&infracfg CLK_INFRA_66M_PWM_CK8>;
+ clock-names = "top", "main", "pwm1", "pwm2", "pwm3",
+ "pwm4", "pwm5", "pwm6", "pwm7", "pwm8";
+ #pwm-cells = <2>;
+ status = "disabled";
+ };
+
usb@11190000 {
compatible = "mediatek,mt7988-xhci", "mediatek,mtk-xhci";
reg = <0 0x11190000 0 0x2e00>,
--
2.35.3
^ permalink raw reply related
* Re: [PATCH v3 6/9] drm: xlnx: zynqmp_dpsub: Set input live format
From: Tomi Valkeinen @ 2024-04-05 12:56 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-6-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Program live video input format according to selected media bus format.
>
> In the bridge mode of operation, DPSUB is connected to FPGA CRTC which
> almost certainly supports a single media bus format as its output. Expect
> this to be delivered via the new bridge atomic state. Program DPSUB
> registers accordingly. Update zynqmp_disp_layer_set_format() API to fit
> both live and non-live layer types.
>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 66 +++++++++++++++++++++++++-------------
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 +++++---
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 0c2b3f4bffa6..a385d22d428e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -436,19 +436,28 @@ static void zynqmp_disp_avbuf_set_format(struct zynqmp_disp *disp,
> const struct zynqmp_disp_format *fmt)
> {
> unsigned int i;
> - u32 val;
> + u32 val, reg;
>
> - val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> - val &= zynqmp_disp_layer_is_video(layer)
> - ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> - : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> - val |= fmt->buf_fmt;
> - zynqmp_disp_avbuf_write(disp, ZYNQMP_DISP_AV_BUF_FMT, val);
> + layer->disp_fmt = fmt;
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
> + reg = ZYNQMP_DISP_AV_BUF_FMT;
> + val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> + val &= zynqmp_disp_layer_is_video(layer)
> + ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> + : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> + val |= fmt->buf_fmt;
> + } else {
> + reg = zynqmp_disp_layer_is_video(layer)
> + ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG
> + : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG;
> + val = fmt->buf_fmt;
> + }
> + zynqmp_disp_avbuf_write(disp, reg, val);
Just write the registers inside the above if-else blocks.
>
> for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; i++) {
> - unsigned int reg = zynqmp_disp_layer_is_video(layer)
> - ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> - : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
> + reg = zynqmp_disp_layer_is_video(layer)
> + ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> + : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
>
> zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]);
> }
> @@ -902,25 +911,33 @@ static void zynqmp_disp_audio_disable(struct zynqmp_disp *disp)
> */
>
> /**
> - * zynqmp_disp_layer_find_format - Find format information for a DRM format
> + * zynqmp_disp_layer_find_format - Find format information for a DRM or media
> + * bus format
> * @layer: The layer
> - * @drm_fmt: DRM format to search
> + * @drm_or_bus_format: DRM or media bus format
> *
> * Search display subsystem format information corresponding to the given DRM
> - * format @drm_fmt for the @layer, and return a pointer to the format
> - * descriptor.
> + * or media bus format @drm_or_bus_format for the @layer, and return a pointer
> + * to the format descriptor. Search key choice depends on @layer mode, for live
> + * layers search is done by zynqmp_disp_format.bus_fmt, and for non-live layers
> + * zynqmp_disp_format.drm_fmt is used.
Here also I recommend creating separate funcs for the fourcc and mbus
versions. They are different types, even if they happen to fit into u32.
> *
> * Return: A pointer to the format descriptor if found, NULL otherwise
> */
> static const struct zynqmp_disp_format *
> zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
> - u32 drm_fmt)
> + u32 drm_or_bus_format)
> {
> unsigned int i;
> + const struct zynqmp_disp_format *disp_format;
>
> for (i = 0; i < layer->info->num_formats; i++) {
> - if (layer->info->formats[i].drm_fmt == drm_fmt)
> - return &layer->info->formats[i];
> + disp_format = &layer->info->formats[i];
> + if ((layer->mode == ZYNQMP_DPSUB_LAYER_LIVE &&
> + disp_format->bus_fmt == drm_or_bus_format) ||
> + (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE &&
> + disp_format->drm_fmt == drm_or_bus_format))
> + return disp_format;
> }
>
> return NULL;
> @@ -992,20 +1009,25 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
> /**
> * zynqmp_disp_layer_set_format - Set the layer format
> * @layer: The layer
> - * @info: The format info
> + * @drm_or_bus_format: DRM or media bus format
> *
> * Set the format for @layer to @info. The layer must be disabled.
> */
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> - const struct drm_format_info *info)
> + u32 drm_or_bus_format)
And here, with a quick look, a separate function would be fine.
Tomi
> {
> unsigned int i;
>
> - layer->disp_fmt = zynqmp_disp_layer_find_format(layer, info->format);
> - layer->drm_fmt = info;
> + layer->disp_fmt = zynqmp_disp_layer_find_format(layer, drm_or_bus_format);
> + if (WARN_ON(!layer->disp_fmt))
> + return;
>
> zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>
> + layer->drm_fmt = drm_format_info(layer->disp_fmt->drm_fmt);
> + if (!layer->drm_fmt)
> + return;
> +
> if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return;
>
> @@ -1013,7 +1035,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> * Set pconfig for each DMA channel to indicate they're part of a
> * video group.
> */
> - for (i = 0; i < info->num_planes; i++) {
> + for (i = 0; i < layer->drm_fmt->num_planes; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
> struct xilinx_dpdma_peripheral_config pconfig = {
> .video_group = true,
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 88c285a12e23..9f9a5f50ffbc 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -55,7 +55,7 @@ u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> - const struct drm_format_info *info);
> + u32 drm_or_bus_format);
> int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
> struct drm_plane_state *state);
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index e3b9eb3d9273..200e63636006 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1299,15 +1299,20 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> struct drm_bridge_state *old_bridge_state)
> {
> struct zynqmp_disp_layer *layer;
> - const struct drm_format_info *info;
> + struct drm_bridge_state *bridge_state;
> + u32 bus_fmt;
>
> layer = zynqmp_dp_disp_connected_live_layer(dp);
> if (!layer)
> return;
>
> - /* TODO: Make the format configurable. */
> - info = drm_format_info(DRM_FORMAT_YUV422);
> - zynqmp_disp_layer_set_format(layer, info);
> + bridge_state = drm_atomic_get_new_bridge_state(old_bridge_state->base.state,
> + old_bridge_state->bridge);
> + if (WARN_ON(!bridge_state))
> + return;
> +
> + bus_fmt = bridge_state->input_bus_cfg.format;
> + zynqmp_disp_layer_set_format(layer, bus_fmt);
> zynqmp_disp_layer_enable(layer);
>
> if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index bf9fba01df0e..d96b3f3f2e3a 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -111,7 +111,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
> if (old_state->fb)
> zynqmp_disp_layer_disable(layer);
>
> - zynqmp_disp_layer_set_format(layer, new_state->fb->format);
> + zynqmp_disp_layer_set_format(layer, new_state->fb->format->format);
> }
>
> zynqmp_disp_layer_update(layer, new_state);
>
^ permalink raw reply
* Re: [RESEND PATCH v9 2/4] dt-bindings: stm32: update DT bingding for stm32mp25
From: Gabriel FERNANDEZ @ 2024-04-05 12:54 UTC (permalink / raw)
To: Krzysztof Kozlowski, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
Alexandre Torgue, Philipp Zabel
Cc: linux-clk, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <e70dc513-df9f-4b99-b9d9-7ebaf83e8f3e@linaro.org>
On 4/5/24 09:12, Krzysztof Kozlowski wrote:
> On 02/04/2024 14:53, gabriel.fernandez@foss.st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>>
>> Now RCC driver use '.index' of clk_parent_data struct to define a parent.
>> The majority of parents are SCMI clocks, then dt-bindings must be fixed.
>>
>> Fixes: b5be49db3d47 ("dt-bindings: stm32: add clocks and reset binding for stm32mp25 platform")
> And except what Rob said, this does not look as a fix. How ABI break
> could be a fix and what is even to fix here? Please describe the
> observable bug, how it manifests itself and what is exactly the fix for
> that bug.
As I replied to Rob, there are no RCC STM32MP25 drivers already upstreamed.
However, in my series, the DT binding was merged even though Stephen
made some
important remarks that needed to be taken into account.
That's why I proposed a fix to update the documentation.
To be sure, how would you like me to proceed?
Best Regards,
Gabriel
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [RESEND PATCH v9 2/4] dt-bindings: stm32: update DT bingding for stm32mp25
From: Gabriel FERNANDEZ @ 2024-04-05 12:54 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd, Krzysztof Kozlowski,
Conor Dooley, Maxime Coquelin, Alexandre Torgue, Philipp Zabel,
linux-clk, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <20240404135201.GA2320777-robh@kernel.org>
On 4/4/24 15:52, Rob Herring wrote:
> On Tue, Apr 02, 2024 at 02:53:10PM +0200, gabriel.fernandez@foss.st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>>
>> Now RCC driver use '.index' of clk_parent_data struct to define a parent.
>> The majority of parents are SCMI clocks, then dt-bindings must be fixed.
> This is an ABI change. Please make that clear and justify why that is
> okay. Changing a driver is not a valid reason. What about other drivers
> besides Linux?
As the SoC STM32MP25X is not yet official and it is not available
outside STMicroelectronics, it is not a issue to have ABI change
and I will upstream the driver in other component (TF-A, U-Boot
and OP-TEE) when binding and driver will be accepted and merged
in Linux repository to avoid binding divergence.
Today no other STM32MP25 RCC drivers are yet upstreamed.
Best Regards,
Gabriel
>> Fixes: b5be49db3d47 ("dt-bindings: stm32: add clocks and reset binding for stm32mp25 platform")
>>
> Should not have a blank line here.
ok
>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>> ---
> Please put version history for a patch within the patch here.
ok
>
>> .../bindings/clock/st,stm32mp25-rcc.yaml | 171 ++++++++++++++++--
>> 1 file changed, 155 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml b/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
>> index 7732e79a42b9..57bd4e7157bd 100644
>> --- a/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
>> @@ -38,22 +38,87 @@ properties:
>> - description: CK_SCMI_MSI Low Power Internal oscillator (~ 4 MHz or ~ 16 MHz)
>> - description: CK_SCMI_LSE Low Speed External oscillator (32 KHz)
>> - description: CK_SCMI_LSI Low Speed Internal oscillator (~ 32 KHz)
>> -
>> - clock-names:
>> - items:
>> - - const: hse
>> - - const: hsi
>> - - const: msi
>> - - const: lse
>> - - const: lsi
>> -
>> + - description: CK_SCMI_HSE_DIV2 CK_SCMI_HSE divided by 2 (coud be gated)
>> + - description: CK_SCMI_ICN_HS_MCU High Speed interconnect bus clock
>> + - description: CK_SCMI_ICN_LS_MCU Low Speed interconnect bus clock
>> + - description: CK_SCMI_ICN_SDMMC SDMMC interconnect bus clock
>> + - description: CK_SCMI_ICN_DDR DDR interconnect bus clock
>> + - description: CK_SCMI_ICN_DISPLAY Display interconnect bus clock
>> + - description: CK_SCMI_ICN_HSL HSL interconnect bus clock
>> + - description: CK_SCMI_ICN_NIC NIC interconnect bus clock
>> + - description: CK_SCMI_ICN_VID Video interconnect bus clock
>> + - description: CK_SCMI_FLEXGEN_07 flexgen clock 7
>> + - description: CK_SCMI_FLEXGEN_08 flexgen clock 8
>> + - description: CK_SCMI_FLEXGEN_09 flexgen clock 9
>> + - description: CK_SCMI_FLEXGEN_10 flexgen clock 10
>> + - description: CK_SCMI_FLEXGEN_11 flexgen clock 11
>> + - description: CK_SCMI_FLEXGEN_12 flexgen clock 12
>> + - description: CK_SCMI_FLEXGEN_13 flexgen clock 13
>> + - description: CK_SCMI_FLEXGEN_14 flexgen clock 14
>> + - description: CK_SCMI_FLEXGEN_15 flexgen clock 15
>> + - description: CK_SCMI_FLEXGEN_16 flexgen clock 16
>> + - description: CK_SCMI_FLEXGEN_17 flexgen clock 17
>> + - description: CK_SCMI_FLEXGEN_18 flexgen clock 18
>> + - description: CK_SCMI_FLEXGEN_19 flexgen clock 19
>> + - description: CK_SCMI_FLEXGEN_20 flexgen clock 20
>> + - description: CK_SCMI_FLEXGEN_21 flexgen clock 21
>> + - description: CK_SCMI_FLEXGEN_22 flexgen clock 22
>> + - description: CK_SCMI_FLEXGEN_23 flexgen clock 23
>> + - description: CK_SCMI_FLEXGEN_24 flexgen clock 24
>> + - description: CK_SCMI_FLEXGEN_25 flexgen clock 25
>> + - description: CK_SCMI_FLEXGEN_26 flexgen clock 26
>> + - description: CK_SCMI_FLEXGEN_27 flexgen clock 27
>> + - description: CK_SCMI_FLEXGEN_28 flexgen clock 28
>> + - description: CK_SCMI_FLEXGEN_29 flexgen clock 29
>> + - description: CK_SCMI_FLEXGEN_30 flexgen clock 30
>> + - description: CK_SCMI_FLEXGEN_31 flexgen clock 31
>> + - description: CK_SCMI_FLEXGEN_32 flexgen clock 32
>> + - description: CK_SCMI_FLEXGEN_33 flexgen clock 33
>> + - description: CK_SCMI_FLEXGEN_34 flexgen clock 34
>> + - description: CK_SCMI_FLEXGEN_35 flexgen clock 35
>> + - description: CK_SCMI_FLEXGEN_36 flexgen clock 36
>> + - description: CK_SCMI_FLEXGEN_37 flexgen clock 37
>> + - description: CK_SCMI_FLEXGEN_38 flexgen clock 38
>> + - description: CK_SCMI_FLEXGEN_39 flexgen clock 39
>> + - description: CK_SCMI_FLEXGEN_40 flexgen clock 40
>> + - description: CK_SCMI_FLEXGEN_41 flexgen clock 41
>> + - description: CK_SCMI_FLEXGEN_42 flexgen clock 42
>> + - description: CK_SCMI_FLEXGEN_43 flexgen clock 43
>> + - description: CK_SCMI_FLEXGEN_44 flexgen clock 44
>> + - description: CK_SCMI_FLEXGEN_45 flexgen clock 45
>> + - description: CK_SCMI_FLEXGEN_46 flexgen clock 46
>> + - description: CK_SCMI_FLEXGEN_47 flexgen clock 47
>> + - description: CK_SCMI_FLEXGEN_48 flexgen clock 48
>> + - description: CK_SCMI_FLEXGEN_49 flexgen clock 49
>> + - description: CK_SCMI_FLEXGEN_50 flexgen clock 50
>> + - description: CK_SCMI_FLEXGEN_51 flexgen clock 51
>> + - description: CK_SCMI_FLEXGEN_52 flexgen clock 52
>> + - description: CK_SCMI_FLEXGEN_53 flexgen clock 53
>> + - description: CK_SCMI_FLEXGEN_54 flexgen clock 54
>> + - description: CK_SCMI_FLEXGEN_55 flexgen clock 55
>> + - description: CK_SCMI_FLEXGEN_56 flexgen clock 56
>> + - description: CK_SCMI_FLEXGEN_57 flexgen clock 57
>> + - description: CK_SCMI_FLEXGEN_58 flexgen clock 58
>> + - description: CK_SCMI_FLEXGEN_59 flexgen clock 59
>> + - description: CK_SCMI_FLEXGEN_60 flexgen clock 60
>> + - description: CK_SCMI_FLEXGEN_61 flexgen clock 61
>> + - description: CK_SCMI_FLEXGEN_62 flexgen clock 62
>> + - description: CK_SCMI_FLEXGEN_63 flexgen clock 63
>> + - description: CK_SCMI_ICN_APB1 Peripheral bridge 1
>> + - description: CK_SCMI_ICN_APB2 Peripheral bridge 2
>> + - description: CK_SCMI_ICN_APB3 Peripheral bridge 3
>> + - description: CK_SCMI_ICN_APB4 Peripheral bridge 4
>> + - description: CK_SCMI_ICN_APBDBG Peripheral bridge for degub
>> + - description: CK_SCMI_TIMG1 Peripheral bridge for timer1
>> + - description: CK_SCMI_TIMG2 Peripheral bridge for timer2
>> + - description: CK_SCMI_PLL3 PLL3 clock
>> + - description: clk_dsi_txbyte DSI byte clock
> Need a blank line here.
>
>> required:
>> - compatible
>> - reg
>> - '#clock-cells'
>> - '#reset-cells'
>> - clocks
>> - - clock-names
>>
>> additionalProperties: false
>>
>> @@ -66,11 +131,85 @@ examples:
>> reg = <0x44200000 0x10000>;
>> #clock-cells = <1>;
>> #reset-cells = <1>;
>> - clock-names = "hse", "hsi", "msi", "lse", "lsi";
>> - clocks = <&scmi_clk CK_SCMI_HSE>,
>> - <&scmi_clk CK_SCMI_HSI>,
>> - <&scmi_clk CK_SCMI_MSI>,
>> - <&scmi_clk CK_SCMI_LSE>,
>> - <&scmi_clk CK_SCMI_LSI>;
>> + clocks = <&scmi_clk CK_SCMI_HSE>,
>> + <&scmi_clk CK_SCMI_HSI>,
>> + <&scmi_clk CK_SCMI_MSI>,
>> + <&scmi_clk CK_SCMI_LSE>,
>> + <&scmi_clk CK_SCMI_LSI>,
>> + <&scmi_clk CK_SCMI_HSE_DIV2>,
>> + <&scmi_clk CK_SCMI_ICN_HS_MCU>,
>> + <&scmi_clk CK_SCMI_ICN_LS_MCU>,
>> + <&scmi_clk CK_SCMI_ICN_SDMMC>,
>> + <&scmi_clk CK_SCMI_ICN_DDR>,
>> + <&scmi_clk CK_SCMI_ICN_DISPLAY>,
>> + <&scmi_clk CK_SCMI_ICN_HSL>,
>> + <&scmi_clk CK_SCMI_ICN_NIC>,
>> + <&scmi_clk CK_SCMI_ICN_VID>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_07>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_08>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_09>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_10>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_11>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_12>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_13>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_14>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_15>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_16>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_17>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_18>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_19>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_20>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_21>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_22>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_23>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_24>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_25>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_26>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_27>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_28>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_29>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_30>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_31>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_32>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_33>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_34>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_35>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_36>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_37>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_38>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_39>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_40>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_41>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_42>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_43>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_44>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_45>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_46>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_47>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_48>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_49>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_50>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_51>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_52>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_53>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_54>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_55>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_56>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_57>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_58>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_59>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_60>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_61>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_62>,
>> + <&scmi_clk CK_SCMI_FLEXGEN_63>,
>> + <&scmi_clk CK_SCMI_ICN_APB1>,
>> + <&scmi_clk CK_SCMI_ICN_APB2>,
>> + <&scmi_clk CK_SCMI_ICN_APB3>,
>> + <&scmi_clk CK_SCMI_ICN_APB4>,
>> + <&scmi_clk CK_SCMI_ICN_APBDBG>,
>> + <&scmi_clk CK_SCMI_TIMG1>,
>> + <&scmi_clk CK_SCMI_TIMG2>,
>> + <&scmi_clk CK_SCMI_PLL3>,
>> + <&clk_dsi_txbyte>;
>> };
>> ...
>> --
>> 2.25.1
>>
^ permalink raw reply
* [PATCH] arm64: dts: mediatek: mt7981: add I2C controller
From: Rafał Miłecki @ 2024-04-05 12:55 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
MT7981 has one on-SoC I2C controller that differs from recent Mediatek
blocks by having a different SLAVE_ADDR register offset (thus a custom
binding compatible string).
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
index 8a6263cc569c..2d7f91196e64 100644
--- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
@@ -94,6 +94,23 @@ pwm@10048000 {
#pwm-cells = <2>;
};
+ i2c@11007000 {
+ compatible = "mediatek,mt7981-i2c";
+ reg = <0 0x11007000 0 0x1000>,
+ <0 0x10217080 0 0x80>;
+ interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&infracfg CLK_INFRA_I2C0_CK>,
+ <&infracfg CLK_INFRA_AP_DMA_CK>,
+ <&infracfg CLK_INFRA_I2C_MCK_CK>,
+ <&infracfg CLK_INFRA_I2C_PCK_CK>;
+ clock-names = "main", "dma", "arb", "pmic";
+ clock-div = <1>;
+ clock-frequency = <100000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
pio: pinctrl@11d00000 {
compatible = "mediatek,mt7981-pinctrl";
reg = <0 0x11d00000 0 0x1000>,
--
2.35.3
^ permalink raw reply related
* Re: [PATCH v5 2/2] iio: adc: adding support for PAC193x
From: Marius.Cristea @ 2024-04-05 12:53 UTC (permalink / raw)
To: jic23, Conor.Dooley
Cc: linux, jdelvare, linux-iio, devicetree, lars, linux-kernel,
linux-hwmon, krzysztof.kozlowski+dt, robh+dt, conor+dt
In-Reply-To: <20240405-embellish-bonnet-ab5f10560d93@wendy>
Hi Conor,
Thanks for reporting the bug. I have detect it and I'm already
working on a patch for it.
Thanks,
Marius
On Fri, 2024-04-05 at 13:41 +0100, Conor Dooley wrote:
> On Sat, Feb 24, 2024 at 07:15:59PM +0000, Jonathan Cameron wrote:
> > On Thu, 22 Feb 2024 18:42:06 +0200
> > <marius.cristea@microchip.com> wrote:
> >
> > > From: Marius Cristea <marius.cristea@microchip.com>
> > >
> > > This is the iio driver for Microchip
> > > PAC193X series of Power Monitor with Accumulator chip family.
> > >
> > > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> > So I had a few comments on this, but nothing that can't be cleaned
> > up later.
> > + I'll fix the thing the bots didn't like on the bindings.
> >
> > Series applied to the togreg branch of iio.git and pushed out
> > as testing for 0-day to take a look at it.
>
> I tested this out on v6.9-rc2 and prompted a backtrace when collectd
> started running:
> ------------[ cut here ]------------
> UBSAN: array-index-out-of-bounds in
> /home/conor/stuff/linux/drivers/iio/adc/pac1934.c:857:25
> index 7 is out of range for type 'u32 [4]'
> CPU: 1 PID: 179 Comm: iiod Not tainted 6.9.0-rc2-dirty #1
> Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> Call Trace:
> [<ffffffff80006bba>] dump_backtrace+0x28/0x30
> [<ffffffff80bd67d8>] show_stack+0x38/0x44
> [<ffffffff80be7820>] dump_stack_lvl+0x6e/0x9a
> [<ffffffff80be7864>] dump_stack+0x18/0x20
> [<ffffffff80be1452>] ubsan_epilogue+0x10/0x46
> [<ffffffff80615358>] __ubsan_handle_out_of_bounds+0x6a/0x78
> [<ffffffff80981f3a>] pac1934_read_raw+0x20c/0x34c
> [<ffffffff80977c4c>] iio_read_channel_info+0x5c/0xbe
> [<ffffffff8073516e>] dev_attr_show+0x1c/0x4a
> [<ffffffff80387292>] sysfs_kf_seq_show+0x80/0xcc
> [<ffffffff80385b12>] kernfs_seq_show+0x3c/0x4a
> [<ffffffff8031e3d8>] seq_read_iter+0x136/0x2e4
> [<ffffffff80385cde>] kernfs_fop_read_iter+0x38/0x16a
> [<ffffffff802e904a>] vfs_read+0x1be/0x2ba
> [<ffffffff802e9c48>] ksys_read+0x64/0xd2
> [<ffffffff802e9cd6>] __riscv_sys_read+0x20/0x28
> [<ffffffff80be838a>] do_trap_ecall_u+0xee/0x204
> [<ffffffff80bf57d0>] ret_from_exception+0x0/0x64
> ---[ end trace ]---
>
> The device itself only has 4 channels, but in sysfs there are "fake"
> channels for the average voltages and currents too. UBSAN points at:
> case PAC1934_VSENSE_AVG_4_ADDR:
> *val = PAC1934_MAX_VSENSE_RSHIFTED_BY_16B;
> if (chan->scan_type.sign == 'u')
> *val2 = info->shunts[channel];
> else
> *val2 = info->shunts[channel] >> 1;
> return IIO_VAL_FRACTIONAL;
>
> And info->shunts is only valid for the 4 real channels, so I guess
> the
> averaged channels probably need to do a [channel - 4] or similar. I
> dunno if that relation between averaged channels and their "real"
> counterparts is fixed or if different pac devices need different
> values,
> but on my system here that would work.
>
> I do quite like that UBSAN points out the line in question :)
>
> Cheers,
> Conor.
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Dmitry Baryshkov @ 2024-04-05 12:49 UTC (permalink / raw)
To: Kalle Valo
Cc: Jeff Johnson, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, ath10k, linux-wireless, netdev,
devicetree, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <87sf002d8d.fsf@kernel.org>
On Fri, 5 Apr 2024 at 15:41, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
> >> >>
> >> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >> >>
> >> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> >> >> hand that means we need to update linux-firmware (basically add a new
> >> >> >> symlink) everytime a new product is added. But are there going to be
> >> >> >> that many new ath10k based products?
> >> >> >>
> >> >> >> Using 'qcm2290' is easier because for a new product then there only
> >> >> >> needs to be a change in DTS and no need to change anything
> >> >> >> linux-firmware. But here the risk is that if there's actually two
> >> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> >> >> directory?
> >> >> >>
> >> >> >> But I don't really know, thoughts?
> >> >> >
> >> >> > After some thought, I'd suggest to follow approach taken by the rest
> >> >> > of qcom firmware:
> >> >>
> >> >> Can you provide pointers to those cases?
> >> >
> >> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
> >> >
> >> >>
> >> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
> >> >> > and then put a vendor-specific firmware into subdir. If any of such
> >> >> > vendors appear, we might even implement structural fallback: first
> >> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> >> >> > finally just under hw1.0.
> >> >>
> >> >> Honestly that looks quite compilicated compared to having just one
> >> >> sub-directory. How will ath10k find the directory names (or I vendor and
> >> >> model names) like 'Google' or 'blueline' in this example?
> >> >
> >> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
> >> > this can be really simpler, firmware-name = "blueline" or
> >> > "sdm845/blueline" with no need for fallbacks.
> >>
> >> I have been also thinking about this and I would prefer not to have the
> >> fallbacks. But good if you agree with that.
> >>
> >> IMHO just "sdm845-blueline" would be the most simple. I don't see the
> >> point of having a directory structure when there are not that many
> >> directories really. But this is just cosmetics.
> >
> > It is "not many directories" if we are thinking about the
> > linux-firmware or open devices. But once embedded distros start
> > picking this up for the supported devices, this can quickly become a
> > nuisance.
>
> Ok. Just out of curiosity, any ideas how many devices/products are there
> with wcn3990 who want to use ath10k?
Just for the DT in mainline I can count about 30 devices that have
ath10k WiFi node:
arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-xiaomi-sagit.dts:&wifi {
arch/arm64/boot/dts/qcom/qcs404-evb.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-idp.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-lenovo-flex-5g.dts:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-primus.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-db845c.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts:&wifi {
arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb2210-rb1.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb4210-rb2.dts:&wifi {
The list doesn't include e.g. PIxel 2-3-4-5 or some other phones which
are still on their way for the proper mainline support.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [RESEND v7 34/37] sh: Add dtbs target support.
From: Geert Uytterhoeven @ 2024-04-05 12:45 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, David Airlie, Daniel Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Thomas Gleixner, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Greg Kroah-Hartman, Jiri Slaby,
Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <f0e220fc338a3dac27f31d7ca871e2ceecad2874.1712207606.git.ysato@users.sourceforge.jp>
On Thu, Apr 4, 2024 at 7:16 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
My
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
on v6 is still valid.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [RESEND v7 33/37] sh: j2_mimas_v2.dts update
From: Geert Uytterhoeven @ 2024-04-05 12:44 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <7cffb0c041744b3c2e324f9908635a912dbb2436.1712207606.git.ysato@users.sourceforge.jp>
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:16 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
From my comments for v6:
Please enhance the one-line summary, e.g.
sh: j2_mimas_v2: Update CPU compatible value
For the actual changes:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 2/3] of: Use scope based kfree() cleanups
From: Rob Herring @ 2024-04-05 12:43 UTC (permalink / raw)
To: Saravana Kannan; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <CAGETcx_H_vvK9y-51JTcz8F7GDThBwC+t=k2i6r4Nst3H6-TUg@mail.gmail.com>
On Thu, Apr 4, 2024 at 6:16 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
> >
> > Use the relatively new scope based kfree() cleanup to simplify error
> > handling. Doing so reduces the chances of memory leaks and simplifies
> > error paths by avoiding the need for goto statements.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > drivers/of/base.c | 34 ++++++++--------------------------
> > drivers/of/dynamic.c | 11 ++++-------
> > drivers/of/resolver.c | 35 +++++++++++++----------------------
> > 3 files changed, 25 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 8856c67c466a..20603d3c9931 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -16,6 +16,7 @@
> >
> > #define pr_fmt(fmt) "OF: " fmt
> >
> > +#include <linux/cleanup.h>
> > #include <linux/console.h>
> > #include <linux/ctype.h>
> > #include <linux/cpu.h>
> > @@ -1393,8 +1394,10 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> > const char *stem_name,
> > int index, struct of_phandle_args *out_args)
> > {
> > - char *cells_name, *map_name = NULL, *mask_name = NULL;
> > - char *pass_name = NULL;
> > + char *cells_name __free(kfree) = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
> > + char *map_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map", stem_name);
> > + char *mask_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-mask", stem_name);
> > + char *pass_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-pass-thru", stem_name);
>
> With the scoped stuff, do these function calls need to be in the same
> line we are defining these variables? If not, I'd rather that the
> calls remain where they were. It feels like a lote to visually parse
> and take in from a readability perspective.
They don't have to be, but if you don't want to get yelled at by the
chief penguin, then yes, they should be together. See the discussions
on adding the scoped iterators. But with the C99 adoption, we can move
the declaration to where the assignment was original.
Rob
^ permalink raw reply
* Re: [PATCH v2 02/18] PCI: endpoint: Introduce pci_epc_map_align()
From: Damien Le Moal @ 2024-04-05 12:43 UTC (permalink / raw)
To: Niklas Cassel
Cc: Kishon Vijay Abraham I, Manivannan Sadhasivam, Lorenzo Pieralisi,
Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-rockchip,
linux-arm-kernel, Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <Zg_skLWnl04-pxkn@ryzen>
On 4/5/24 21:20, Niklas Cassel wrote:
> On Thu, Apr 04, 2024 at 11:43:47AM +0900, Damien Le Moal wrote:
>> On 4/3/24 21:33, Kishon Vijay Abraham I wrote:
>>> Hi Damien,
>>>
>>> On 3/30/2024 9:49 AM, Damien Le Moal wrote:
>>>> Some endpoint controllers have requirements on the alignment of the
>>>> controller physical memory address that must be used to map a RC PCI
>>>> address region. For instance, the rockchip endpoint controller uses
>>>> at most the lower 20 bits of a physical memory address region as the
>>>> lower bits of an RC PCI address. For mapping a PCI address region of
>>>> size bytes starting from pci_addr, the exact number of address bits
>>>> used is the number of address bits changing in the address range
>>>> [pci_addr..pci_addr + size - 1].
>>>>
>>>> For this example, this creates the following constraints:
>>>> 1) The offset into the controller physical memory allocated for a
>>>> mapping depends on the mapping size *and* the starting PCI address
>>>> for the mapping.
>>>> 2) A mapping size cannot exceed the controller windows size (1MB) minus
>>>> the offset needed into the allocated physical memory, which can end
>>>> up being a smaller size than the desired mapping size.
>>>>
>>>> Handling these constraints independently of the controller being used in
>>>> a PCI EP function driver is not possible with the current EPC API as
>>>> it only provides the ->align field in struct pci_epc_features.
>>>> Furthermore, this alignment is static and does not depend on a mapping
>>>> pci address and size.
>>>>
>>>> Solve this by introducing the function pci_epc_map_align() and the
>>>> endpoint controller operation ->map_align to allow endpoint function
>>>> drivers to obtain the size and the offset into a controller address
>>>> region that must be used to map an RC PCI address region. The size
>>>> of the physical address region provided by pci_epc_map_align() can then
>>>> be used as the size argument for the function pci_epc_mem_alloc_addr().
>>>> The offset into the allocated controller memory can be used to
>>>> correctly handle data transfers. Of note is that pci_epc_map_align() may
>>>> indicate upon return a mapping size that is smaller (but not 0) than the
>>>> requested PCI address region size. For such case, an endpoint function
>>>> driver must handle data transfers in fragments.
>>>>
>>>> The controller operation ->map_align is optional: controllers that do
>>>> not have any address alignment constraints for mapping a RC PCI address
>>>> region do not need to implement this operation. For such controllers,
>>>> pci_epc_map_align() always returns the mapping size as equal
>>>> to the requested size and an offset equal to 0.
>>>>
>>>> The structure pci_epc_map is introduced to represent a mapping start PCI
>>>> address, size and the size and offset into the controller memory needed
>>>> for mapping the PCI address region.
>>>>
>>>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>>>> ---
>>>> drivers/pci/endpoint/pci-epc-core.c | 66 +++++++++++++++++++++++++++++
>>>> include/linux/pci-epc.h | 33 +++++++++++++++
>>>> 2 files changed, 99 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>>>> index 754afd115bbd..37758ca91d7f 100644
>>>> --- a/drivers/pci/endpoint/pci-epc-core.c
>>>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>>>> @@ -433,6 +433,72 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>>>> }
>>>> EXPORT_SYMBOL_GPL(pci_epc_unmap_addr);
>>>>
>>>> +/**
>>>> + * pci_epc_map_align() - Get the offset into and the size of a controller memory
>>>> + * address region needed to map a RC PCI address region
>>>> + * @epc: the EPC device on which address is allocated
>>>> + * @func_no: the physical endpoint function number in the EPC device
>>>> + * @vfunc_no: the virtual endpoint function number in the physical function
>>>> + * @pci_addr: PCI address to which the physical address should be mapped
>>>> + * @size: the size of the mapping starting from @pci_addr
>>>> + * @map: populate here the actual size and offset into the controller memory
>>>> + * that must be allocated for the mapping
>>>> + *
>>>> + * Invoke the controller map_align operation to obtain the size and the offset
>>>> + * into a controller address region that must be allocated to map @size
>>>> + * bytes of the RC PCI address space starting from @pci_addr.
>>>> + *
>>>> + * The size of the mapping that can be handled by the controller is indicated
>>>> + * using the pci_size field of @map. This size may be smaller than the requested
>>>> + * @size. In such case, the function driver must handle the mapping using
>>>> + * several fragments. The offset into the controller memory for the effective
>>>> + * mapping of the @pci_addr..@pci_addr+@map->pci_size address range is indicated
>>>> + * using the map_ofst field of @map.
>>>> + */
>>>> +int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>>>> + u64 pci_addr, size_t size, struct pci_epc_map *map)
>>>> +{
>>>> + const struct pci_epc_features *features;
>>>> + size_t mask;
>>>> + int ret;
>>>> +
>>>> + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
>>>> + return -EINVAL;
>>>> +
>>>> + if (!size || !map)
>>>> + return -EINVAL;
>>>> +
>>>> + memset(map, 0, sizeof(*map));
>>>> + map->pci_addr = pci_addr;
>>>> + map->pci_size = size;
>>>> +
>>>> + if (epc->ops->map_align) {
>>>> + mutex_lock(&epc->lock);
>>>> + ret = epc->ops->map_align(epc, func_no, vfunc_no, map);
>>>> + mutex_unlock(&epc->lock);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Assume a fixed alignment constraint as specified by the controller
>>>> + * features.
>>>> + */
>>>> + features = pci_epc_get_features(epc, func_no, vfunc_no);
>>>> + if (!features || !features->align) {
>>>> + map->map_pci_addr = pci_addr;
>>>> + map->map_size = size;
>>>> + map->map_ofst = 0;
>>>> + }
>>>
>>> The 'align' of pci_epc_features was initially added only to address the
>>> inbound ATU constraints. This is also added as comment in [1]. The PCI
>>> address restrictions (only fixed alignment constraint) were handled by
>>> the host side driver and depends on the connected endpoint device
>>> (atleast it was like that for pci_endpoint_test.c [2]).
>>> So pci-epf-test.c used the 'align' in pci_epc_features only as part of
>>> pci_epf_alloc_space().
>>>
>>> Though I have abused 'align' of pci_epc_features in pci-epf-ntb.c using
>>> it out of pci_epf_alloc_space(), I think we should keep the 'align' of
>>> pci_epc_features only within pci_epf_alloc_space() and controllers with
>>> any PCI address restrictions to implement ->map_align(). This could as
>>> well be done in a phased manner to let controllers implement
>>> ->map_align() and then remove using pci_epc_features in
>>> pci_epc_map_align(). Let me know what you think?
>
> First you say that you want to avoid using epc_features->align inside
> pci_epc_map_align(), and then you say that we could do it in phases,
> and eventually stop using epc_features->align in pci_epc_map_align().
>
> I'm confused... :)
>
> Do you really want pci_epc_map_align() to make use of epc_features->align ?
>
> Don't you mean ep->page_size ?
> (Please read the whole email to see my reasoning.)
>
>
>>
>> Yep, good idea. I will remove the use of "align" as a default alignment
>> constraint. For controllers that have a fixed alignment constraint (not
>> necessarilly epc->features->align), it is trivial to provide a generic helper
>> function that implements the ->map_align method.
>
> We can see that commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2a9a801620efac92885fc9cd53594c0b9aba87a4
>
> Introduced epc_features->align and modified pci_epf_alloc_space() to use it.
>
> From reading the commit, it appears that epc_features->align was intended to
> represent inbound iATU alignment requirement.
>
> For DWC based controllers, the inbound iATU address must be aligned to:
> CX_ATU_MIN_REGION_SIZE.
>
> AFAICT, epc_features->align currently has nothing to do with traffic outbound
> from the EP.
Yes, correct. It is for BARs, not for PCI address alignment constraint.
> For aligning the reads/writes to buffers allocated on the host side,
> we currently have .alignment in the host side driver:
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L966-L1021
>
> Which should be set to the outbound iATU alignment requirement.
>
> For DWC based controllers, the outbound iATU address must be aligned to:
> CX_ATU_MIN_REGION_SIZE.
> Additionally, we have ep->page_size, which defines the smallest outbound unit
> that can be mapped.
> (On DWC based controllers, tis is CX_ATU_MIN_REGION_SIZE.)
>
> ep->page_size is used to specify the outbound alignment for e.g.
> dw_pcie_ep_raise_msi_irq() and dw_pcie_ep_raise_msix_irq():
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/pci/controller/dwc/pcie-designware-ep.c#L488
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/pci/controller/dwc/pcie-designware-ep.c#L555> which makes sure that we can write to the RC side MSI/MSI-X address
> while satisfying the outbound iATU alignment requirement.
>
> See also:
> https://lore.kernel.org/linux-pci/20240402-pci2_upstream-v3-2-803414bdb430@nxp.com/
>
>
>
> Now I understand that rockchip is the first one that does not have a fixed
> alignment.
> So for that platform, map_align() will be different from ep->page_size.
> (For all DWC based drivers the outbound iATU alignment requirement is
> the same as the page size.)
Yes. So we can have a generic map_align() implementation that all these drivers
can use as there .map_align method. No need to expose page size to the epc/epf
core code.
> However, it would be nice if:
> 1) We could have a default implementation of map_align() that by default uses
> ep->page_size. Platforms that have non-fixed alignment requirements could
> define their own map_align().
See above. The default implementation can be a helper function defined in epc
core that the drivers can use for their .map_align() method.
>
> 2) We fix dw_pcie_ep_raise_msi_irq() and dw_pcie_ep_raise_msix_irq() to use
> the new pci_epc_map_align().
Why ? That is completely internal to the controller driver.
>
> 3) It is getting too complicated with all these...
> epc_features->align, ep->page_size, map_align(), and .alignment in host driver.
> I think that we need to document each of these in Documentation/PCI/endpoint/
test host driver .alignment needs to be nuked. That one is nonsense.
ep->page_size needs to stay internal to the driver. .map_align method is enough
to handle any PCI address mapping constraint and will indicate memory size to
allocate, offset into it etc. And for the BARs alignment, .align feature is not
exactly great as it is not clear, but it is enough I think. So we could just
rename it to be clear. And even maybe remove it from features. I do not see why
an EPF needs to care about it given that epc core funstions are used to setup
the bars.
> 4) It would be nice if we could set page_size correctly for all the PCI device
> and vendor IDs that have defined an .alignment in drivers/misc/pci_endpoint_test.c
> in the correct EPC driver. That way, we should be able to completely remove all
> .alignment specified in drivers/misc/pci_endpoint_test.c.
The host side should be allowed to use any PCI address alignment it wants. So no
alignment communicated at all. It is the EP side that needs to deal with alignment.
> 5) Unfortunately drivers/misc/pci_endpoint_test.c defines a default alignment
> of 4K:
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L968
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L820
>
> It would be nice if we could get rid of this as well. Or perhaps add an option
> to pci_test so that it does not use this 4k alignment, such that we can verify
> that pci_epc_map_align() is actually working.
Exactly. Get rid of any default alignment, add a test parameter to define one so
that we can test different alignment+size combinations.
> In my opinion 4) is the biggest win with this series, because it means that
> we define the alignment in the EPC driver, instead of needing to define it in
> each and every host side driver. But right now, this great improvement is not
> really visible for someone looking quickly at the current series.
Yes. Once in place, we can rework the test driver alignment stuff to make it
optional instead of mandatory because of bad handling on the EP side :)
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH v5 2/2] iio: adc: adding support for PAC193x
From: Conor Dooley @ 2024-04-05 12:41 UTC (permalink / raw)
To: Jonathan Cameron
Cc: marius.cristea, lars, robh+dt, jdelvare, linux, linux-hwmon,
krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20240224191559.40d233db@jic23-huawei>
[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]
On Sat, Feb 24, 2024 at 07:15:59PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 18:42:06 +0200
> <marius.cristea@microchip.com> wrote:
>
> > From: Marius Cristea <marius.cristea@microchip.com>
> >
> > This is the iio driver for Microchip
> > PAC193X series of Power Monitor with Accumulator chip family.
> >
> > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> So I had a few comments on this, but nothing that can't be cleaned up later.
> + I'll fix the thing the bots didn't like on the bindings.
>
> Series applied to the togreg branch of iio.git and pushed out
> as testing for 0-day to take a look at it.
I tested this out on v6.9-rc2 and prompted a backtrace when collectd
started running:
------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in /home/conor/stuff/linux/drivers/iio/adc/pac1934.c:857:25
index 7 is out of range for type 'u32 [4]'
CPU: 1 PID: 179 Comm: iiod Not tainted 6.9.0-rc2-dirty #1
Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
Call Trace:
[<ffffffff80006bba>] dump_backtrace+0x28/0x30
[<ffffffff80bd67d8>] show_stack+0x38/0x44
[<ffffffff80be7820>] dump_stack_lvl+0x6e/0x9a
[<ffffffff80be7864>] dump_stack+0x18/0x20
[<ffffffff80be1452>] ubsan_epilogue+0x10/0x46
[<ffffffff80615358>] __ubsan_handle_out_of_bounds+0x6a/0x78
[<ffffffff80981f3a>] pac1934_read_raw+0x20c/0x34c
[<ffffffff80977c4c>] iio_read_channel_info+0x5c/0xbe
[<ffffffff8073516e>] dev_attr_show+0x1c/0x4a
[<ffffffff80387292>] sysfs_kf_seq_show+0x80/0xcc
[<ffffffff80385b12>] kernfs_seq_show+0x3c/0x4a
[<ffffffff8031e3d8>] seq_read_iter+0x136/0x2e4
[<ffffffff80385cde>] kernfs_fop_read_iter+0x38/0x16a
[<ffffffff802e904a>] vfs_read+0x1be/0x2ba
[<ffffffff802e9c48>] ksys_read+0x64/0xd2
[<ffffffff802e9cd6>] __riscv_sys_read+0x20/0x28
[<ffffffff80be838a>] do_trap_ecall_u+0xee/0x204
[<ffffffff80bf57d0>] ret_from_exception+0x0/0x64
---[ end trace ]---
The device itself only has 4 channels, but in sysfs there are "fake"
channels for the average voltages and currents too. UBSAN points at:
case PAC1934_VSENSE_AVG_4_ADDR:
*val = PAC1934_MAX_VSENSE_RSHIFTED_BY_16B;
if (chan->scan_type.sign == 'u')
*val2 = info->shunts[channel];
else
*val2 = info->shunts[channel] >> 1;
return IIO_VAL_FRACTIONAL;
And info->shunts is only valid for the 4 real channels, so I guess the
averaged channels probably need to do a [channel - 4] or similar. I
dunno if that relation between averaged channels and their "real"
counterparts is fixed or if different pac devices need different values,
but on my system here that would work.
I do quite like that UBSAN points out the line in question :)
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/9] drm: xlnx: zynqmp_dpsub: Minimize usage of global flag
From: Tomi Valkeinen @ 2024-04-05 12:42 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-5-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Avoid usage of global zynqmp_dpsub.dma_enabled flag in DPSUB layer
> context. This flag signals whether the driver runs in DRM CRTC or DRM
> bridge mode, assuming that all display layers share the same live or
> non-live mode of operation. Using per-layer mode instead of global flag
> will simplify future support of the hybrid scenario.
>
> Remove redundant checks in DMA request/release contexts as
> zynqmp_disp_layer.info is well-defined for all layer types, including the
> correct number of DMA channels required for each particular layer.
>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index abdc3971b193..0c2b3f4bffa6 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -980,7 +980,7 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
> {
> unsigned int i;
>
> - if (layer->disp->dpsub->dma_enabled) {
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
> for (i = 0; i < layer->drm_fmt->num_planes; i++)
> dmaengine_terminate_sync(layer->dmas[i].chan);
> }
> @@ -1006,7 +1006,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
>
> zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>
> - if (!layer->disp->dpsub->dma_enabled)
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return;
>
> /*
> @@ -1044,7 +1044,7 @@ int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
> const struct drm_format_info *info = layer->drm_fmt;
> unsigned int i;
>
> - if (!layer->disp->dpsub->dma_enabled)
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return 0;
>
> for (i = 0; i < info->num_planes; i++) {
> @@ -1094,9 +1094,6 @@ static void zynqmp_disp_layer_release_dma(struct zynqmp_disp *disp,
> {
> unsigned int i;
>
> - if (!layer->info || !disp->dpsub->dma_enabled)
> - return;
> -
> for (i = 0; i < layer->info->num_channels; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
>
> @@ -1137,9 +1134,6 @@ static int zynqmp_disp_layer_request_dma(struct zynqmp_disp *disp,
> unsigned int i;
> int ret;
>
> - if (!disp->dpsub->dma_enabled)
> - return 0;
> -
> for (i = 0; i < layer->info->num_channels; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
> char dma_channel_name[16];
>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Kalle Valo @ 2024-04-05 12:41 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jeff Johnson, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, ath10k, linux-wireless, netdev,
devicetree, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <CAA8EJppASEmj6-Jt7OCABAeqr8umSgXaDDha9nn2nRafuZ-Gvw@mail.gmail.com>
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>>
>> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
>> >>
>> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>> >>
>> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
>> >> >> hand that means we need to update linux-firmware (basically add a new
>> >> >> symlink) everytime a new product is added. But are there going to be
>> >> >> that many new ath10k based products?
>> >> >>
>> >> >> Using 'qcm2290' is easier because for a new product then there only
>> >> >> needs to be a change in DTS and no need to change anything
>> >> >> linux-firmware. But here the risk is that if there's actually two
>> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
>> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
>> >> >> directory?
>> >> >>
>> >> >> But I don't really know, thoughts?
>> >> >
>> >> > After some thought, I'd suggest to follow approach taken by the rest
>> >> > of qcom firmware:
>> >>
>> >> Can you provide pointers to those cases?
>> >
>> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
>> >
>> >>
>> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
>> >> > and then put a vendor-specific firmware into subdir. If any of such
>> >> > vendors appear, we might even implement structural fallback: first
>> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
>> >> > finally just under hw1.0.
>> >>
>> >> Honestly that looks quite compilicated compared to having just one
>> >> sub-directory. How will ath10k find the directory names (or I vendor and
>> >> model names) like 'Google' or 'blueline' in this example?
>> >
>> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
>> > this can be really simpler, firmware-name = "blueline" or
>> > "sdm845/blueline" with no need for fallbacks.
>>
>> I have been also thinking about this and I would prefer not to have the
>> fallbacks. But good if you agree with that.
>>
>> IMHO just "sdm845-blueline" would be the most simple. I don't see the
>> point of having a directory structure when there are not that many
>> directories really. But this is just cosmetics.
>
> It is "not many directories" if we are thinking about the
> linux-firmware or open devices. But once embedded distros start
> picking this up for the supported devices, this can quickly become a
> nuisance.
Ok. Just out of curiosity, any ideas how many devices/products are there
with wcn3990 who want to use ath10k?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v3 4/9] drm: xlnx: zynqmp_dpsub: Anounce supported input formats
From: Tomi Valkeinen @ 2024-04-05 12:39 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-4-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> DPSUB in bridge mode supports multiple input media bus formats.
>
> Announce the list of supported input media bus formats via
> drm_bridge.atomic_get_input_bus_fmts callback.
> Introduce a set of live input formats, supported by DPSUB.
> Rename zynqmp_disp_layer_drm_formats() to zynqmp_disp_layer_formats() to
> reflect semantics for both live and non-live layer format lists.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 76 +++++++++++++++++++++++++++++++++-----
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 31 ++++++++++++++++
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 101 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index e6d26ef60e89..abdc3971b193 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -18,6 +18,7 @@
> #include <linux/dma/xilinx_dpdma.h>
> #include <linux/dma-mapping.h>
> #include <linux/dmaengine.h>
> +#include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> @@ -77,12 +78,14 @@ enum zynqmp_dpsub_layer_mode {
> /**
> * struct zynqmp_disp_format - Display subsystem format information
> * @drm_fmt: DRM format (4CC)
> + * @bus_fmt: Media bus format
> * @buf_fmt: AV buffer format
> * @swap: Flag to swap R & B for RGB formats, and U & V for YUV formats
> * @sf: Scaling factors for color components
> */
> struct zynqmp_disp_format {
> u32 drm_fmt;
> + u32 bus_fmt;
> u32 buf_fmt;
> bool swap;
> const u32 *sf;
> @@ -182,6 +185,12 @@ static const u32 scaling_factors_565[] = {
> ZYNQMP_DISP_AV_BUF_5BIT_SF,
> };
>
> +static const u32 scaling_factors_666[] = {
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> +};
> +
> static const u32 scaling_factors_888[] = {
> ZYNQMP_DISP_AV_BUF_8BIT_SF,
> ZYNQMP_DISP_AV_BUF_8BIT_SF,
> @@ -364,6 +373,41 @@ static const struct zynqmp_disp_format avbuf_gfx_fmts[] = {
> },
> };
>
> +/* List of live video layer formats */
> +static const struct zynqmp_disp_format avbuf_live_fmts[] = {
> + {
> + .drm_fmt = DRM_FORMAT_RGB565,
> + .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_6 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> + .sf = scaling_factors_666,
> + }, {
> + .drm_fmt = DRM_FORMAT_RGB888,
> + .bus_fmt = MEDIA_BUS_FMT_RGB888_1X24,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_YUV422,
> + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_YUV444,
> + .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_P210,
> + .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
> + .sf = scaling_factors_101010,
> + },
> +};
> +
> static u32 zynqmp_disp_avbuf_read(struct zynqmp_disp *disp, int reg)
> {
> return readl(disp->avbuf.base + reg);
> @@ -883,16 +927,17 @@ zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
> }
>
> /**
> - * zynqmp_disp_layer_drm_formats - Return the DRM formats supported by the layer
> + * zynqmp_disp_layer_formats - Return DRM or media bus formats supported by
> + * the layer
> * @layer: The layer
> * @num_formats: Pointer to the returned number of formats
> *
> - * Return: A newly allocated u32 array that stores all the DRM formats
> + * Return: A newly allocated u32 array that stores all DRM or media bus formats
> * supported by the layer. The number of formats in the array is returned
> * through the num_formats argument.
> */
> -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> - unsigned int *num_formats)
> +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> + unsigned int *num_formats)
> {
> unsigned int i;
> u32 *formats;
> @@ -903,7 +948,9 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> return NULL;
>
> for (i = 0; i < layer->info->num_formats; ++i)
> - formats[i] = layer->info->formats[i].drm_fmt;
> + formats[i] = layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE
> + ? layer->info->formats[i].drm_fmt
> + : layer->info->formats[i].bus_fmt;
I find this quite confusing. Depending on the layer mode, you return
different format types. I think it's quite easy to use this kind of
function the wrong way.
Why not just make two separate functions?
Tomi
> *num_formats = layer->info->num_formats;
> return formats;
> @@ -1131,6 +1178,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
> .num_channels = 1,
> },
> };
> + static const struct zynqmp_disp_layer_info live_layer_info = {
> + .formats = avbuf_live_fmts,
> + .num_formats = ARRAY_SIZE(avbuf_live_fmts),
> + .num_channels = 0,
> + };
>
> unsigned int i;
> int ret;
> @@ -1140,12 +1192,18 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
>
> layer->id = i;
> layer->disp = disp;
> - layer->info = &layer_info[i];
> - /* For now assume dpsub works in either live or non-live mode for both layers.
> +
> + /*
> + * For now assume dpsub works in either live or non-live mode for both layers.
> * Hybrid mode is not supported yet.
> */
> - layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
> - : ZYNQMP_DPSUB_LAYER_LIVE;
> + if (disp->dpsub->dma_enabled) {
> + layer->mode = ZYNQMP_DPSUB_LAYER_NONLIVE;
> + layer->info = &layer_info[i];
> + } else {
> + layer->mode = ZYNQMP_DPSUB_LAYER_LIVE;
> + layer->info = &live_layer_info;
> + }
>
> ret = zynqmp_disp_layer_request_dma(disp, layer);
> if (ret)
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 9b8b202224d9..88c285a12e23 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -50,8 +50,8 @@ int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
> bool enable, u32 alpha);
>
> -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> - unsigned int *num_formats);
> +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> + unsigned int *num_formats);
> void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 4faafdd76798..e3b9eb3d9273 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -22,6 +22,7 @@
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/io.h>
> +#include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> @@ -1577,6 +1578,35 @@ static const struct drm_edid *zynqmp_dp_bridge_edid_read(struct drm_bridge *brid
> return drm_edid_read_ddc(connector, &dp->aux.ddc);
> }
>
> +static u32 *zynqmp_dp_bridge_default_bus_fmts(unsigned int *num_input_fmts)
> +{
> + u32 *formats = kzalloc(sizeof(*formats), GFP_KERNEL);
> +
> + if (formats)
> + *formats = MEDIA_BUS_FMT_FIXED;
> + *num_input_fmts = !!formats;
> +
> + return formats;
> +}
> +
> +static u32 *
> +zynqmp_dp_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int *num_input_fmts)
> +{
> + struct zynqmp_dp *dp = bridge_to_dp(bridge);
> + struct zynqmp_disp_layer *layer;
> +
> + layer = zynqmp_dp_disp_connected_live_layer(dp);
> + if (layer)
> + return zynqmp_disp_layer_formats(layer, num_input_fmts);
> + else
> + return zynqmp_dp_bridge_default_bus_fmts(num_input_fmts);
> +}
> +
> static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .attach = zynqmp_dp_bridge_attach,
> .detach = zynqmp_dp_bridge_detach,
> @@ -1589,6 +1619,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .atomic_check = zynqmp_dp_bridge_atomic_check,
> .detect = zynqmp_dp_bridge_detect,
> .edid_read = zynqmp_dp_bridge_edid_read,
> + .atomic_get_input_bus_fmts = zynqmp_dp_bridge_get_input_bus_fmts,
> };
>
> /* -----------------------------------------------------------------------------
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index 43bf416b33d5..bf9fba01df0e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -152,7 +152,7 @@ static int zynqmp_dpsub_create_planes(struct zynqmp_dpsub *dpsub)
> unsigned int num_formats;
> u32 *formats;
>
> - formats = zynqmp_disp_layer_drm_formats(layer, &num_formats);
> + formats = zynqmp_disp_layer_formats(layer, &num_formats);
> if (!formats)
> return -ENOMEM;
>
>
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Dmitry Baryshkov @ 2024-04-05 12:34 UTC (permalink / raw)
To: Kalle Valo
Cc: Jeff Johnson, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, ath10k, linux-wireless, netdev,
devicetree, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <871q7k3tnq.fsf@kernel.org>
On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> >> hand that means we need to update linux-firmware (basically add a new
> >> >> symlink) everytime a new product is added. But are there going to be
> >> >> that many new ath10k based products?
> >> >>
> >> >> Using 'qcm2290' is easier because for a new product then there only
> >> >> needs to be a change in DTS and no need to change anything
> >> >> linux-firmware. But here the risk is that if there's actually two
> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> >> directory?
> >> >>
> >> >> But I don't really know, thoughts?
> >> >
> >> > After some thought, I'd suggest to follow approach taken by the rest
> >> > of qcom firmware:
> >>
> >> Can you provide pointers to those cases?
> >
> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
> >
> >>
> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
> >> > and then put a vendor-specific firmware into subdir. If any of such
> >> > vendors appear, we might even implement structural fallback: first
> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> >> > finally just under hw1.0.
> >>
> >> Honestly that looks quite compilicated compared to having just one
> >> sub-directory. How will ath10k find the directory names (or I vendor and
> >> model names) like 'Google' or 'blueline' in this example?
> >
> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
> > this can be really simpler, firmware-name = "blueline" or
> > "sdm845/blueline" with no need for fallbacks.
>
> I have been also thinking about this and I would prefer not to have the
> fallbacks. But good if you agree with that.
>
> IMHO just "sdm845-blueline" would be the most simple. I don't see the
> point of having a directory structure when there are not that many
> directories really. But this is just cosmetics.
It is "not many directories" if we are thinking about the
linux-firmware or open devices. But once embedded distros start
picking this up for the supported devices, this can quickly become a
nuisance. We have been there for Qualcomm DSP firmware and we ended up
adopting the SoC/vendor/device structure, because otherwise it becomes
a bedlam.
> > My point is that the firmware-name provides the possibility to handle
> > that in different ways.
>
> Very good, thanks.
Thanks for your suggestions and for picking the patches.
Bjorn, could you please pick up the DT patches now?
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [RESEND v7 09/37] dt-binding: Add compatible SH7750 SoC
From: Geert Uytterhoeven @ 2024-04-05 12:31 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, David Airlie, Daniel Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Thomas Gleixner, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Greg Kroah-Hartman, Jiri Slaby,
Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <4ac65d0f311e890c1ca92bf057c70954ec7ac351.1712207606.git.ysato@users.sourceforge.jp>
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v2 6/6] firmware: imx: add i.MX95 MISC driver
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The i.MX95 System manager exports SCMI MISC protocol for linux to do
various settings, such as set board gpio expander as wakeup source.
The driver is to add the support.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/Makefile | 1 +
drivers/firmware/imx/sm-misc.c | 92 +++++++++++++++++++++++++++++++++++++++++
include/linux/firmware/imx/sm.h | 33 +++++++++++++++
3 files changed, 126 insertions(+)
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index fb20e22074e1..cb9c361d9b81 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
+obj-${CONFIG_IMX_SCMI_MISC_EXT} += sm-misc.o
diff --git a/drivers/firmware/imx/sm-misc.c b/drivers/firmware/imx/sm-misc.c
new file mode 100644
index 000000000000..a5609de426f6
--- /dev/null
+++ b/drivers/firmware/imx/sm-misc.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/firmware/imx/sm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+static const struct scmi_imx_misc_proto_ops *imx_misc_ctrl_ops;
+static struct scmi_protocol_handle *ph;
+struct notifier_block scmi_imx_misc_ctrl_nb;
+
+int scmi_imx_misc_ctrl_set(u32 id, u32 val)
+{
+ if (!ph)
+ return -EPROBE_DEFER;
+
+ return imx_misc_ctrl_ops->misc_ctrl_set(ph, id, 1, &val);
+};
+EXPORT_SYMBOL(scmi_imx_misc_ctrl_set);
+
+int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
+{
+ if (!ph)
+ return -EPROBE_DEFER;
+
+ return imx_misc_ctrl_ops->misc_ctrl_get(ph, id, num, val);
+}
+EXPORT_SYMBOL(scmi_imx_misc_ctrl_get);
+
+static int scmi_imx_misc_ctrl_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ return 0;
+}
+
+static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device_node *np = sdev->dev.of_node;
+ u32 src_id, evt_id, wu_num;
+ int ret, i;
+
+ if (!handle)
+ return -ENODEV;
+
+ imx_misc_ctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_MISC, &ph);
+ if (IS_ERR(imx_misc_ctrl_ops))
+ return PTR_ERR(imx_misc_ctrl_ops);
+
+ scmi_imx_misc_ctrl_nb.notifier_call = &scmi_imx_misc_ctrl_notifier;
+ wu_num = of_property_count_u32_elems(np, "wakeup-sources");
+ if (wu_num % 2) {
+ dev_err(&sdev->dev, "Invalid wakeup-sources\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < wu_num; i += 2) {
+ WARN_ON(of_property_read_u32_index(np, "wakeup-sources", i, &src_id));
+ WARN_ON(of_property_read_u32_index(np, "wakeup-sources", i + 1, &evt_id));
+ ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_MISC,
+ evt_id,
+ &src_id,
+ &scmi_imx_misc_ctrl_nb);
+ if (ret)
+ dev_err(&sdev->dev, "Failed to register scmi misc event: %d\n", src_id);
+ }
+
+ return 0;
+
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_IMX_MISC, "imx-misc-ctrl" },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_imx_misc_ctrl_driver = {
+ .name = "scmi-imx-misc-ctrl",
+ .probe = scmi_imx_misc_ctrl_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_imx_misc_ctrl_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("IMX SM MISC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
new file mode 100644
index 000000000000..daad4bdf7d1c
--- /dev/null
+++ b/include/linux/firmware/imx/sm.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef _SCMI_IMX_H
+#define _SCMI_IMX_H
+
+#include <linux/bitfield.h>
+#include <linux/types.h>
+
+#define SCMI_IMX_CTRL_PDM_CLK_SEL 0 /* AON PDM clock sel */
+#define SCMI_IMX_CTRL_MQS1_SETTINGS 1 /* AON MQS settings */
+#define SCMI_IMX_CTRL_SAI1_MCLK 2 /* AON SAI1 MCLK */
+#define SCMI_IMX_CTRL_SAI3_MCLK 3 /* WAKE SAI3 MCLK */
+#define SCMI_IMX_CTRL_SAI4_MCLK 4 /* WAKE SAI4 MCLK */
+#define SCMI_IMX_CTRL_SAI5_MCLK 5 /* WAKE SAI5 MCLK */
+
+#if IS_ENABLED(CONFIG_IMX_SCMI_MISC_EXT)
+int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val);
+int scmi_imx_misc_ctrl_set(u32 id, u32 val);
+#else
+static inline int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_misc_ctrl_set(u32 id, u32 val);
+{
+ return -EOPNOTSUPP;
+}
+#endif
+#endif
--
2.37.1
^ permalink raw reply related
* [PATCH v2 5/6] firmware: imx: support BBM module
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The BBM module provides RTC and BUTTON feature. To i.MX95, this module
is managed by System Manager. Linux could use i.MX SCMI BBM Extension
protocol to use RTC and BUTTON feature.
This driver is to use SCMI interface to get/set RTC, enable pwrkey.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/Makefile | 1 +
drivers/firmware/imx/sm-bbm.c | 317 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 318 insertions(+)
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index 8f9f04a513a8..fb20e22074e1 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
+obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
diff --git a/drivers/firmware/imx/sm-bbm.c b/drivers/firmware/imx/sm-bbm.c
new file mode 100644
index 000000000000..fcb2ae8490c8
--- /dev/null
+++ b/drivers/firmware/imx/sm-bbm.c
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+#include <linux/suspend.h>
+
+#define DEBOUNCE_TIME 30
+#define REPEAT_INTERVAL 60
+
+struct scmi_imx_bbm {
+ struct rtc_device *rtc_dev;
+ struct scmi_protocol_handle *ph;
+ const struct scmi_imx_bbm_proto_ops *ops;
+ struct notifier_block nb;
+ int keycode;
+ int keystate; /* 1:pressed */
+ bool suspended;
+ struct delayed_work check_work;
+ struct input_dev *input;
+};
+
+static int scmi_imx_bbm_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ u64 val;
+ int ret;
+
+ ret = bbnsm->ops->rtc_time_get(ph, 0, &val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ rtc_time64_to_tm(val, tm);
+
+ return 0;
+}
+
+static int scmi_imx_bbm_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ u64 val;
+ int ret;
+
+ val = rtc_tm_to_time64(tm);
+
+ ret = bbnsm->ops->rtc_time_set(ph, 0, val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ return 0;
+}
+
+static int scmi_imx_bbm_alarm_irq_enable(struct device *dev, unsigned int enable)
+{
+ return 0;
+}
+
+static int scmi_imx_bbm_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ struct rtc_time *alrm_tm = &alrm->time;
+ u64 val;
+ int ret;
+
+ val = rtc_tm_to_time64(alrm_tm);
+
+ ret = bbnsm->ops->rtc_alarm_set(ph, 0, val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ return 0;
+}
+
+static const struct rtc_class_ops smci_imx_bbm_rtc_ops = {
+ .read_time = scmi_imx_bbm_read_time,
+ .set_time = scmi_imx_bbm_set_time,
+ .set_alarm = scmi_imx_bbm_set_alarm,
+ .alarm_irq_enable = scmi_imx_bbm_alarm_irq_enable,
+};
+
+static void scmi_imx_bbm_pwrkey_check_for_events(struct work_struct *work)
+{
+ struct scmi_imx_bbm *bbnsm = container_of(work, struct scmi_imx_bbm, check_work.work);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ struct input_dev *input = bbnsm->input;
+ u32 state = 0;
+ int ret;
+
+ ret = bbnsm->ops->button_get(ph, &state);
+ if (ret) {
+ pr_err("%s: %d\n", __func__, ret);
+ return;
+ }
+
+ pr_debug("%s: state: %d, keystate %d\n", __func__, state, bbnsm->keystate);
+
+ /* only report new event if status changed */
+ if (state ^ bbnsm->keystate) {
+ bbnsm->keystate = state;
+ input_event(input, EV_KEY, bbnsm->keycode, state);
+ input_sync(input);
+ pm_relax(bbnsm->input->dev.parent);
+ pr_debug("EV_KEY: %x\n", bbnsm->keycode);
+ }
+
+ /* repeat check if pressed long */
+ if (state)
+ schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(REPEAT_INTERVAL));
+}
+
+static int scmi_imx_bbm_pwrkey_event(struct scmi_imx_bbm *bbnsm)
+{
+ struct input_dev *input = bbnsm->input;
+
+ schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(DEBOUNCE_TIME));
+
+ /*
+ * Directly report key event after resume to make no key press
+ * event is missed.
+ */
+ if (bbnsm->suspended) {
+ bbnsm->keystate = 1;
+ input_event(input, EV_KEY, bbnsm->keycode, 1);
+ input_sync(input);
+ }
+
+ return 0;
+}
+
+static void scmi_imx_bbm_pwrkey_act(void *pdata)
+{
+ struct scmi_imx_bbm *bbnsm = pdata;
+
+ cancel_delayed_work_sync(&bbnsm->check_work);
+}
+
+static int scmi_imx_bbm_notifier(struct notifier_block *nb, unsigned long event, void *data)
+{
+ struct scmi_imx_bbm *bbnsm = container_of(nb, struct scmi_imx_bbm, nb);
+ struct scmi_imx_bbm_notif_report *r = data;
+
+ if (r->is_rtc)
+ rtc_update_irq(bbnsm->rtc_dev, 1, RTC_AF | RTC_IRQF);
+ if (r->is_button) {
+ pr_debug("BBM Button Power key pressed\n");
+ scmi_imx_bbm_pwrkey_event(bbnsm);
+ }
+
+ return 0;
+}
+
+static int scmi_imx_bbm_pwrkey_init(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct input_dev *input;
+ int ret;
+
+ if (device_property_read_u32(dev, "linux,code", &bbnsm->keycode)) {
+ bbnsm->keycode = KEY_POWER;
+ dev_warn(dev, "key code is not specified, using default KEY_POWER\n");
+ }
+
+ INIT_DELAYED_WORK(&bbnsm->check_work, scmi_imx_bbm_pwrkey_check_for_events);
+
+ input = devm_input_allocate_device(dev);
+ if (!input) {
+ dev_err(dev, "failed to allocate the input device for SCMI IMX BBM\n");
+ return -ENOMEM;
+ }
+
+ input->name = dev_name(dev);
+ input->phys = "bbnsm-pwrkey/input0";
+ input->id.bustype = BUS_HOST;
+
+ input_set_capability(input, EV_KEY, bbnsm->keycode);
+
+ ret = devm_add_action_or_reset(dev, scmi_imx_bbm_pwrkey_act, bbnsm);
+ if (ret) {
+ dev_err(dev, "failed to register remove action\n");
+ return ret;
+ }
+
+ bbnsm->input = input;
+
+ ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
+ SCMI_EVENT_IMX_BBM_BUTTON,
+ NULL, &bbnsm->nb);
+
+ if (ret)
+ dev_err(dev, "Failed to register BBM Button Events %d:", ret);
+
+ ret = input_register_device(input);
+ if (ret) {
+ dev_err(dev, "failed to register input device\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int scmi_imx_bbm_rtc_init(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ int ret;
+
+ bbnsm->rtc_dev = devm_rtc_allocate_device(dev);
+ if (IS_ERR(bbnsm->rtc_dev))
+ return PTR_ERR(bbnsm->rtc_dev);
+
+ bbnsm->rtc_dev->ops = &smci_imx_bbm_rtc_ops;
+ bbnsm->rtc_dev->range_min = 0;
+ bbnsm->rtc_dev->range_max = U32_MAX;
+
+ ret = devm_rtc_register_device(bbnsm->rtc_dev);
+ if (ret)
+ return ret;
+
+ bbnsm->nb.notifier_call = &scmi_imx_bbm_notifier;
+ return handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
+ SCMI_EVENT_IMX_BBM_RTC,
+ NULL, &bbnsm->nb);
+}
+
+static int scmi_imx_bbm_probe(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_protocol_handle *ph;
+ struct scmi_imx_bbm *bbnsm;
+ int ret;
+
+ if (!handle)
+ return -ENODEV;
+
+ bbnsm = devm_kzalloc(dev, sizeof(struct scmi_imx_bbm), GFP_KERNEL);
+ if (!bbnsm)
+ return -ENOMEM;
+
+ bbnsm->ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_BBM, &ph);
+ if (IS_ERR(bbnsm->ops))
+ return PTR_ERR(bbnsm->ops);
+
+ bbnsm->ph = ph;
+
+ device_init_wakeup(dev, true);
+
+ dev_set_drvdata(dev, bbnsm);
+
+ ret = scmi_imx_bbm_rtc_init(sdev);
+ if (ret) {
+ dev_err(dev, "rtc init failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = scmi_imx_bbm_pwrkey_init(sdev);
+ if (ret) {
+ dev_err(dev, "pwr init failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused scmi_imx_bbm_suspend(struct device *dev)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+ bbnsm->suspended = true;
+
+ return 0;
+}
+
+static int __maybe_unused scmi_imx_bbm_resume(struct device *dev)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+ bbnsm->suspended = false;
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(scmi_imx_bbm_pm_ops, scmi_imx_bbm_suspend, scmi_imx_bbm_resume);
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_IMX_BBM, "imx-bbm" },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_imx_bbm_driver = {
+ .driver = {
+ .pm = &scmi_imx_bbm_pm_ops,
+ },
+ .name = "scmi-imx-bbm",
+ .probe = scmi_imx_bbm_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_imx_bbm_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("IMX SM BBM driver");
+MODULE_LICENSE("GPL");
--
2.37.1
^ permalink raw reply related
* [PATCH v2 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The i.MX MISC protocol is for misc settings, such as gpio expander
wakeup.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/Kconfig | 10 ++
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/imx-sm-misc.c | 305 ++++++++++++++++++++++++++++++++
include/linux/scmi_imx_protocol.h | 17 ++
4 files changed, 333 insertions(+)
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index 56d11c9d9f47..bfeae92f6420 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -191,3 +191,13 @@ config IMX_SCMI_BBM_EXT
and BUTTON.
This driver can also be built as a module.
+
+config IMX_SCMI_MISC_EXT
+ tristate "i.MX SCMI MISC EXTENSION"
+ depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
+ default y if ARCH_MXC
+ help
+ This enables i.MX System MISC control logic such as gpio expander
+ wakeup
+
+ This driver can also be built as a module.
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 327687acf857..a23fde721222 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -12,6 +12,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
scmi-protocols-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
+scmi-protocols-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
diff --git a/drivers/firmware/arm_scmi/imx-sm-misc.c b/drivers/firmware/arm_scmi/imx-sm-misc.c
new file mode 100644
index 000000000000..1b0ec2281518
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx-sm-misc.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System control and Management Interface (SCMI) NXP MISC Protocol
+ *
+ * Copyright 2024 NXP
+ */
+
+#define pr_fmt(fmt) "SCMI Notifications MISC - " fmt
+
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+#include "protocols.h"
+#include "notify.h"
+
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
+
+enum scmi_imx_misc_protocol_cmd {
+ SCMI_IMX_MISC_CTRL_SET = 0x3,
+ SCMI_IMX_MISC_CTRL_GET = 0x4,
+ SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
+};
+
+struct scmi_imx_misc_info {
+ u32 version;
+ u32 nr_dev_ctrl;
+ u32 nr_brd_ctrl;
+ u32 nr_reason;
+};
+
+struct scmi_msg_imx_misc_protocol_attributes {
+ __le32 attributes;
+};
+
+#define GET_BRD_CTRLS_NR(x) le32_get_bits((x), GENMASK(31, 24))
+#define GET_REASONS_NR(x) le32_get_bits((x), GENMASK(23, 16))
+#define GET_DEV_CTRLS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+#define BRD_CTRL_START_ID BIT(15)
+
+struct scmi_imx_misc_ctrl_set_in {
+ __le32 id;
+ __le32 num;
+ __le32 value[MISC_MAX_VAL];
+};
+
+struct scmi_imx_misc_ctrl_notify_in {
+ __le32 ctrl_id;
+ __le32 flags;
+};
+
+struct scmi_imx_misc_ctrl_notify_payld {
+ __le32 ctrl_id;
+ __le32 flags;
+};
+
+struct scmi_imx_misc_ctrl_get_out {
+ __le32 num;
+ __le32 *val;
+};
+
+static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_imx_misc_info *mi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_imx_misc_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0,
+ sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ mi->nr_dev_ctrl = GET_DEV_CTRLS_NR(attr->attributes);
+ mi->nr_brd_ctrl = GET_BRD_CTRLS_NR(attr->attributes);
+ mi->nr_reason = GET_REASONS_NR(attr->attributes);
+ dev_info(ph->dev, "i.MX MISC NUM DEV CTRL: %d, NUM BRD CTRL: %d,NUM Reason: %d\n",
+ mi->nr_dev_ctrl, mi->nr_brd_ctrl, mi->nr_reason);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_validate_id(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id)
+{
+ struct scmi_imx_misc_info *mi = ph->get_priv(ph);
+
+ if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi->nr_dev_ctrl))
+ return -EINVAL;
+ if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int scmi_imx_misc_ctrl_notify(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 flags)
+{
+ struct scmi_imx_misc_ctrl_notify_in *in;
+ struct scmi_xfer *t;
+ int ret;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_NOTIFY,
+ sizeof(*in), 0, &t);
+ if (ret)
+ return ret;
+
+ in = t->tx.buf;
+ in->ctrl_id = cpu_to_le32(ctrl_id);
+ in->flags = cpu_to_le32(flags);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int
+scmi_imx_misc_ctrl_set_notify_enabled(const struct scmi_protocol_handle *ph,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_imx_misc_ctrl_notify(ph, src_id, enable ? evt_id : 0);
+ if (ret)
+ dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] - ret:%d\n",
+ evt_id, src_id, ret);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_get_num_sources(const struct scmi_protocol_handle *ph)
+{
+ return GENMASK(15, 0);
+}
+
+static void *
+scmi_imx_misc_ctrl_fill_custom_report(const struct scmi_protocol_handle *ph,
+ u8 evt_id, ktime_t timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
+ struct scmi_imx_misc_ctrl_notify_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ return NULL;
+
+ r->timestamp = timestamp;
+ r->ctrl_id = p->ctrl_id;
+ r->flags = p->flags;
+ *src_id = r->ctrl_id;
+ dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
+ r->ctrl_id, r->flags);
+
+ return r;
+}
+
+static const struct scmi_event_ops scmi_imx_misc_event_ops = {
+ .get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
+ .set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
+ .fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
+};
+
+static const struct scmi_event scmi_imx_misc_events[] = {
+ {
+ .id = SCMI_EVENT_IMX_MISC_CONTROL_DISABLED,
+ .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
+ },
+ {
+ .id = SCMI_EVENT_IMX_MISC_CONTROL_FALLING_EDGE,
+ .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
+ },
+ {
+ .id = SCMI_EVENT_IMX_MISC_CONTROL_RISING_EDGE,
+ .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
+ }
+};
+
+static struct scmi_protocol_events scmi_imx_misc_protocol_events = {
+ .queue_sz = SCMI_PROTO_QUEUE_SZ,
+ .ops = &scmi_imx_misc_event_ops,
+ .evts = scmi_imx_misc_events,
+ .num_events = ARRAY_SIZE(scmi_imx_misc_events),
+};
+
+static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ struct scmi_imx_misc_info *minfo;
+ u32 version;
+ int ret;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
+ if (!minfo)
+ return -ENOMEM;
+
+ ret = scmi_imx_misc_attributes_get(ph, minfo);
+ if (ret)
+ return ret;
+
+ return ph->set_priv(ph, minfo, version);
+}
+
+static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 *num, u32 *val)
+{
+ struct scmi_imx_misc_ctrl_get_out *out;
+ struct scmi_xfer *t;
+ int ret, i;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET, sizeof(u32),
+ 0, &t);
+ if (ret)
+ return ret;
+
+ put_unaligned_le32(ctrl_id, t->tx.buf);
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ out = t->rx.buf;
+ *num = le32_to_cpu(out->num);
+ for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
+ val[i] = le32_to_cpu(out->val[i]);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 num, u32 *val)
+{
+ struct scmi_imx_misc_ctrl_set_in *in;
+ struct scmi_xfer *t;
+ int ret, i;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ if (num > MISC_MAX_VAL)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET, sizeof(*in),
+ 0, &t);
+ if (ret)
+ return ret;
+
+ in = t->tx.buf;
+ in->id = cpu_to_le32(ctrl_id);
+ in->num = cpu_to_le32(num);
+ for (i = 0; i < num; i++)
+ in->value[i] = cpu_to_le32(val[i]);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
+ .misc_ctrl_set = scmi_imx_misc_ctrl_set,
+ .misc_ctrl_get = scmi_imx_misc_ctrl_get,
+};
+
+static const struct scmi_protocol scmi_imx_misc = {
+ .id = SCMI_PROTOCOL_IMX_MISC,
+ .owner = THIS_MODULE,
+ .instance_init = &scmi_imx_misc_protocol_init,
+ .ops = &scmi_imx_misc_proto_ops,
+ .events = &scmi_imx_misc_protocol_events,
+ .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
+};
+module_scmi_protocol(scmi_imx_misc);
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
index 90ce011a4429..a69bd4a20f0f 100644
--- a/include/linux/scmi_imx_protocol.h
+++ b/include/linux/scmi_imx_protocol.h
@@ -13,8 +13,14 @@
#include <linux/notifier.h>
#include <linux/types.h>
+#define SCMI_PAYLOAD_LEN 100
+
+#define SCMI_ARRAY(X, Y) ((SCMI_PAYLOAD_LEN - (X)) / sizeof(Y))
+#define MISC_MAX_VAL SCMI_ARRAY(8, uint32_t)
+
enum scmi_nxp_protocol {
SCMI_PROTOCOL_IMX_BBM = 0x81,
+ SCMI_PROTOCOL_IMX_MISC = 0x84,
};
struct scmi_imx_bbm_proto_ops {
@@ -42,4 +48,15 @@ struct scmi_imx_bbm_notif_report {
unsigned int rtc_id;
unsigned int rtc_evt;
};
+
+struct scmi_imx_misc_ctrl_notify_report {
+ ktime_t timestamp;
+ unsigned int ctrl_id;
+ unsigned int flags;
+};
+
+struct scmi_imx_misc_proto_ops {
+ int (*misc_ctrl_set)(const struct scmi_protocol_handle *ph, u32 id, u32 num, u32 *val);
+ int (*misc_ctrl_get)(const struct scmi_protocol_handle *ph, u32 id, u32 *num, u32 *val);
+};
#endif
--
2.37.1
^ permalink raw reply related
* [PATCH v2 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The i.MX BBM protocol is for managing i.MX BBM module which provides
RTC and BUTTON feature.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/Kconfig | 10 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/imx-sm-bbm.c | 378 +++++++++++++++++++++++++++++++++
include/linux/scmi_imx_protocol.h | 45 ++++
4 files changed, 434 insertions(+)
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index aa5842be19b2..56d11c9d9f47 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -181,3 +181,13 @@ config ARM_SCMI_POWER_CONTROL
early shutdown/reboot SCMI requests.
endmenu
+
+config IMX_SCMI_BBM_EXT
+ tristate "i.MX SCMI BBM EXTENSION"
+ depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
+ default y if ARCH_MXC
+ help
+ This enables i.MX System BBM control logic which supports RTC
+ and BUTTON.
+
+ This driver can also be built as a module.
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index a7bc4796519c..327687acf857 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -11,6 +11,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
+scmi-protocols-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
diff --git a/drivers/firmware/arm_scmi/imx-sm-bbm.c b/drivers/firmware/arm_scmi/imx-sm-bbm.c
new file mode 100644
index 000000000000..92c0aedf65cc
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx-sm-bbm.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) NXP BBM Protocol
+ *
+ * Copyright 2024 NXP
+ */
+
+#define pr_fmt(fmt) "SCMI Notifications BBM - " fmt
+
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+#include "protocols.h"
+#include "notify.h"
+
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
+
+enum scmi_imx_bbm_protocol_cmd {
+ IMX_BBM_GPR_SET = 0x3,
+ IMX_BBM_GPR_GET = 0x4,
+ IMX_BBM_RTC_ATTRIBUTES = 0x5,
+ IMX_BBM_RTC_TIME_SET = 0x6,
+ IMX_BBM_RTC_TIME_GET = 0x7,
+ IMX_BBM_RTC_ALARM_SET = 0x8,
+ IMX_BBM_BUTTON_GET = 0x9,
+ IMX_BBM_RTC_NOTIFY = 0xA,
+ IMX_BBM_BUTTON_NOTIFY = 0xB,
+};
+
+#define GET_RTCS_NR(x) le32_get_bits((x), GENMASK(23, 16))
+#define GET_GPRS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+
+#define SCMI_IMX_BBM_NOTIFY_RTC_UPDATED BIT(2)
+#define SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER BIT(1)
+#define SCMI_IMX_BBM_NOTIFY_RTC_ALARM BIT(0)
+
+#define SCMI_IMX_BBM_RTC_ALARM_ENABLE_FLAG BIT(0)
+
+#define SCMI_IMX_BBM_NOTIFY_RTC_FLAG \
+ (SCMI_IMX_BBM_NOTIFY_RTC_UPDATED | SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER | \
+ SCMI_IMX_BBM_NOTIFY_RTC_ALARM)
+
+#define SCMI_IMX_BBM_EVENT_RTC_MASK GENMASK(31, 24)
+
+struct scmi_imx_bbm_info {
+ u32 version;
+ int nr_rtc;
+ int nr_gpr;
+};
+
+struct scmi_msg_imx_bbm_protocol_attributes {
+ __le32 attributes;
+};
+
+struct scmi_imx_bbm_set_time {
+ __le32 id;
+ __le32 flags;
+ __le32 value_low;
+ __le32 value_high;
+};
+
+struct scmi_imx_bbm_get_time {
+ __le32 id;
+ __le32 flags;
+};
+
+struct scmi_imx_bbm_alarm_time {
+ __le32 id;
+ __le32 flags;
+ __le32 value_low;
+ __le32 value_high;
+};
+
+struct scmi_msg_imx_bbm_rtc_notify {
+ __le32 rtc_id;
+ __le32 flags;
+};
+
+struct scmi_msg_imx_bbm_button_notify {
+ __le32 flags;
+};
+
+struct scmi_imx_bbm_notify_payld {
+ __le32 flags;
+};
+
+static int scmi_imx_bbm_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_imx_bbm_info *pi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_imx_bbm_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ pi->nr_rtc = GET_RTCS_NR(attr->attributes);
+ pi->nr_gpr = GET_GPRS_NR(attr->attributes);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_notify(const struct scmi_protocol_handle *ph,
+ u32 src_id, int message_id, bool enable)
+{
+ int ret;
+ struct scmi_xfer *t;
+
+ if (message_id == IMX_BBM_RTC_NOTIFY) {
+ struct scmi_msg_imx_bbm_rtc_notify *rtc_notify;
+
+ ret = ph->xops->xfer_get_init(ph, message_id,
+ sizeof(*rtc_notify), 0, &t);
+ if (ret)
+ return ret;
+
+ rtc_notify = t->tx.buf;
+ rtc_notify->rtc_id = cpu_to_le32(0);
+ rtc_notify->flags =
+ cpu_to_le32(enable ? SCMI_IMX_BBM_NOTIFY_RTC_FLAG : 0);
+ } else if (message_id == IMX_BBM_BUTTON_NOTIFY) {
+ struct scmi_msg_imx_bbm_button_notify *button_notify;
+
+ ret = ph->xops->xfer_get_init(ph, message_id,
+ sizeof(*button_notify), 0, &t);
+ if (ret)
+ return ret;
+
+ button_notify = t->tx.buf;
+ button_notify->flags = cpu_to_le32(enable ? 1 : 0);
+ } else {
+ return -EINVAL;
+ }
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+ return ret;
+}
+
+static enum scmi_imx_bbm_protocol_cmd evt_2_cmd[] = {
+ IMX_BBM_RTC_NOTIFY,
+ IMX_BBM_BUTTON_NOTIFY
+};
+
+static int scmi_imx_bbm_set_notify_enabled(const struct scmi_protocol_handle *ph,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret, cmd_id;
+
+ if (evt_id >= ARRAY_SIZE(evt_2_cmd))
+ return -EINVAL;
+
+ cmd_id = evt_2_cmd[evt_id];
+ ret = scmi_imx_bbm_notify(ph, src_id, cmd_id, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+ evt_id, src_id, ret);
+
+ return ret;
+}
+
+static void *scmi_imx_bbm_fill_custom_report(const struct scmi_protocol_handle *ph,
+ u8 evt_id, ktime_t timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_imx_bbm_notify_payld *p = payld;
+ struct scmi_imx_bbm_notif_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ return NULL;
+
+ if (evt_id == SCMI_EVENT_IMX_BBM_RTC) {
+ r->is_rtc = true;
+ r->is_button = false;
+ r->timestamp = timestamp;
+ r->rtc_id = le32_get_bits(p->flags, SCMI_IMX_BBM_EVENT_RTC_MASK);
+ r->rtc_evt = le32_get_bits(p->flags, SCMI_IMX_BBM_NOTIFY_RTC_FLAG);
+ dev_dbg(ph->dev, "RTC: %d evt: %x\n", r->rtc_id, r->rtc_evt);
+ *src_id = r->rtc_evt;
+ } else if (evt_id == SCMI_EVENT_IMX_BBM_BUTTON) {
+ r->is_rtc = false;
+ r->is_button = true;
+ r->timestamp = timestamp;
+ dev_dbg(ph->dev, "BBM Button\n");
+ *src_id = 0;
+ } else {
+ WARN_ON_ONCE(1);
+ return NULL;
+ }
+
+ return r;
+}
+
+static const struct scmi_event scmi_imx_bbm_events[] = {
+ {
+ .id = SCMI_EVENT_IMX_BBM_RTC,
+ .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
+ },
+ {
+ .id = SCMI_EVENT_IMX_BBM_BUTTON,
+ .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
+ },
+};
+
+static const struct scmi_event_ops scmi_imx_bbm_event_ops = {
+ .set_notify_enabled = scmi_imx_bbm_set_notify_enabled,
+ .fill_custom_report = scmi_imx_bbm_fill_custom_report,
+};
+
+static const struct scmi_protocol_events scmi_imx_bbm_protocol_events = {
+ .queue_sz = SCMI_PROTO_QUEUE_SZ,
+ .ops = &scmi_imx_bbm_event_ops,
+ .evts = scmi_imx_bbm_events,
+ .num_events = ARRAY_SIZE(scmi_imx_bbm_events),
+ .num_sources = 1,
+};
+
+static int scmi_imx_bbm_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ u32 version;
+ int ret;
+ struct scmi_imx_bbm_info *binfo;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_info(ph->dev, "NXP SM BBM Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ binfo = devm_kzalloc(ph->dev, sizeof(*binfo), GFP_KERNEL);
+ if (!binfo)
+ return -ENOMEM;
+
+ ret = scmi_imx_bbm_attributes_get(ph, binfo);
+ if (ret)
+ return ret;
+
+ return ph->set_priv(ph, binfo, version);
+}
+
+static int scmi_imx_bbm_rtc_time_set(const struct scmi_protocol_handle *ph,
+ u32 rtc_id, u64 sec)
+{
+ struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+ struct scmi_imx_bbm_set_time *cfg;
+ struct scmi_xfer *t;
+ int ret;
+
+ if (rtc_id >= pi->nr_rtc)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_TIME_SET, sizeof(*cfg), 0, &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(rtc_id);
+ cfg->flags = 0;
+ cfg->value_low = lower_32_bits(sec);
+ cfg->value_high = upper_32_bits(sec);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_rtc_time_get(const struct scmi_protocol_handle *ph,
+ u32 rtc_id, u64 *value)
+{
+ struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+ struct scmi_imx_bbm_get_time *cfg;
+ struct scmi_xfer *t;
+ int ret;
+
+ if (rtc_id >= pi->nr_rtc)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_TIME_GET, sizeof(*cfg),
+ sizeof(u64), &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(rtc_id);
+ cfg->flags = 0;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret)
+ *value = get_unaligned_le64(t->rx.buf);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_rtc_alarm_set(const struct scmi_protocol_handle *ph,
+ u32 rtc_id, u64 sec)
+{
+ struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+ struct scmi_imx_bbm_alarm_time *cfg;
+ struct scmi_xfer *t;
+ int ret;
+
+ if (rtc_id >= pi->nr_rtc)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_ALARM_SET, sizeof(*cfg), 0, &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(rtc_id);
+ cfg->flags = SCMI_IMX_BBM_RTC_ALARM_ENABLE_FLAG;
+ cfg->value_low = lower_32_bits(sec);
+ cfg->value_high = upper_32_bits(sec);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_button_get(const struct scmi_protocol_handle *ph, u32 *state)
+{
+ struct scmi_xfer *t;
+ int ret;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_BUTTON_GET, 0, sizeof(u32), &t);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret)
+ *state = get_unaligned_le32(t->rx.buf);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static const struct scmi_imx_bbm_proto_ops scmi_imx_bbm_proto_ops = {
+ .rtc_time_get = scmi_imx_bbm_rtc_time_get,
+ .rtc_time_set = scmi_imx_bbm_rtc_time_set,
+ .rtc_alarm_set = scmi_imx_bbm_rtc_alarm_set,
+ .button_get = scmi_imx_bbm_button_get,
+};
+
+static const struct scmi_protocol scmi_imx_bbm = {
+ .id = SCMI_PROTOCOL_IMX_BBM,
+ .owner = THIS_MODULE,
+ .instance_init = &scmi_imx_bbm_protocol_init,
+ .ops = &scmi_imx_bbm_proto_ops,
+ .events = &scmi_imx_bbm_protocol_events,
+ .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
+};
+
+module_scmi_protocol(scmi_imx_bbm);
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
new file mode 100644
index 000000000000..90ce011a4429
--- /dev/null
+++ b/include/linux/scmi_imx_protocol.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * SCMI Message Protocol driver NXP extension header
+ *
+ * Copyright 2024 NXP.
+ */
+
+#ifndef _LINUX_SCMI_NXP_PROTOCOL_H
+#define _LINUX_SCMI_NXP_PROTOCOL_H
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/notifier.h>
+#include <linux/types.h>
+
+enum scmi_nxp_protocol {
+ SCMI_PROTOCOL_IMX_BBM = 0x81,
+};
+
+struct scmi_imx_bbm_proto_ops {
+ int (*rtc_time_set)(const struct scmi_protocol_handle *ph, u32 id,
+ uint64_t sec);
+ int (*rtc_time_get)(const struct scmi_protocol_handle *ph, u32 id,
+ u64 *val);
+ int (*rtc_alarm_set)(const struct scmi_protocol_handle *ph, u32 id,
+ u64 sec);
+ int (*button_get)(const struct scmi_protocol_handle *ph, u32 *state);
+};
+
+enum scmi_nxp_notification_events {
+ SCMI_EVENT_IMX_BBM_RTC = 0x0,
+ SCMI_EVENT_IMX_BBM_BUTTON = 0x1,
+ SCMI_EVENT_IMX_MISC_CONTROL_DISABLED = 0x0,
+ SCMI_EVENT_IMX_MISC_CONTROL_FALLING_EDGE = 0x1,
+ SCMI_EVENT_IMX_MISC_CONTROL_RISING_EDGE = 0x2,
+};
+
+struct scmi_imx_bbm_notif_report {
+ bool is_rtc;
+ bool is_button;
+ ktime_t timestamp;
+ unsigned int rtc_id;
+ unsigned int rtc_evt;
+};
+#endif
--
2.37.1
^ permalink raw reply related
* [PATCH v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension protocol
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
Add i.MX SCMI Extension protocols bindings for:
- Battery Backed Secure Module(BBSM)
- MISC settings such as General Purpose Registers settings.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
.../devicetree/bindings/firmware/imx,scmi.yaml | 80 ++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/imx,scmi.yaml b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
new file mode 100644
index 000000000000..7ee19a661d83
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/imx,scmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX System Control and Management Interface(SCMI) Vendor Protocols Extension
+
+maintainers:
+ - Peng Fan <peng.fan@nxp.com>
+
+allOf:
+ - $ref: arm,scmi.yaml#
+
+properties:
+ protocol@81:
+ $ref: 'arm,scmi.yaml#/$defs/protocol-node'
+ unevaluatedProperties: false
+ description:
+ The BBM Protocol is for managing Battery Backed Secure Module (BBSM) RTC
+ and the ON/OFF Key
+
+ properties:
+ reg:
+ const: 0x81
+
+ required:
+ - reg
+
+ protocol@84:
+ $ref: 'arm,scmi.yaml#/$defs/protocol-node'
+ unevaluatedProperties: false
+ description:
+ The MISC Protocol is for managing SoC Misc settings, such as GPR settings
+
+ properties:
+ reg:
+ const: 0x84
+
+ wakeup-sources:
+ description:
+ Each entry consists of 2 integers, represents the source and electric signal edge
+ items:
+ items:
+ - description: the wakeup source
+ - description: the wakeup electric signal edge
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+
+ required:
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ firmware {
+ scmi {
+ compatible = "arm,scmi";
+ mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
+ shmem = <&scmi_buf0>, <&scmi_buf1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ protocol@81 {
+ reg = <0x81>;
+ };
+
+ protocol@84 {
+ reg = <0x84>;
+ wakeup-sources = <0x8000 1
+ 0x8001 1
+ 0x8002 1
+ 0x8003 1
+ 0x8004 1>;
+ };
+ };
+ };
+...
--
2.37.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox