* [PATCH 2/3] dt-bindings: pinctrl: mediatek: mt7622: add "antsel" function
From: Rafał Miłecki @ 2024-04-08 9:05 UTC (permalink / raw)
To: Sean Wang, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Hsin-Yi Wang,
linux-mediatek, linux-gpio, devicetree, linux-arm-kernel,
Rafał Miłecki, Chuanhong Guo
In-Reply-To: <20240408090506.21596-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
MT7622 pinctrl has ANTSEL* pins. Linux support for those was added in
the commit 19f599e83ac5 ("pinctrl: mediatek: mt7622: add antsel
pins/groups"). Include them in binding.
Cc: Chuanhong Guo <gch981213@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../pinctrl/mediatek,mt7622-pinctrl.yaml | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
index 60f30a59f385..a9ab623b2697 100644
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
@@ -75,8 +75,8 @@ patternProperties:
function:
description:
A string containing the name of the function to mux to the group.
- enum: [emmc, eth, i2c, i2s, ir, led, flash, pcie, pmic, pwm, sd,
- spi, tdm, uart, watchdog, wifi]
+ enum: [antsel, emmc, eth, i2c, i2s, ir, led, flash, pcie, pmic, pwm,
+ sd, spi, tdm, uart, watchdog, wifi]
groups:
description:
@@ -90,6 +90,20 @@ patternProperties:
- function
allOf:
+ - if:
+ properties:
+ function:
+ const: antsel
+ then:
+ properties:
+ groups:
+ items:
+ enum: [antsel0, antsel1, antsel2, antsel3, antsel4, antsel5,
+ antsel6, antsel7, antsel8, antsel9, antsel10,
+ antsel11, antsel12, antsel13, antsel14, antsel15,
+ antsel16, antsel17, antsel18, antsel19, antsel20,
+ antsel21, antsel22, antsel23, antsel24, antsel25,
+ antsel26, antsel27, antsel28, antsel29]
- if:
properties:
function:
--
2.35.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 3/3] dt-bindings: pinctrl: mediatek: mt7622: add "base" reg
From: Rafał Miłecki @ 2024-04-08 9:05 UTC (permalink / raw)
To: Sean Wang, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Hsin-Yi Wang,
linux-mediatek, linux-gpio, devicetree, linux-arm-kernel,
Rafał Miłecki
In-Reply-To: <20240408090506.21596-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
Both: MT7622 and MT7629 use two reg spaces: "base" and "eint". It wasn't
covered by the binding and also example didn't match incomplete binding.
Fix it.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../bindings/pinctrl/mediatek,mt7622-pinctrl.yaml | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
index a9ab623b2697..eaa65512161a 100644
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
@@ -19,10 +19,12 @@ properties:
- mediatek,mt7629-pinctrl
reg:
- maxItems: 1
+ minItems: 2
+ maxItems: 2
reg-names:
items:
+ - const: base
- const: eint
gpio-controller: true
@@ -364,7 +366,9 @@ examples:
pio: pinctrl@10211000 {
compatible = "mediatek,mt7622-pinctrl";
- reg = <0 0x10211000 0 0x1000>;
+ reg = <0 0x10211000 0 0x1000>,
+ <0 0x10005000 0 0x1000>;
+ reg-names = "base", "eint";
gpio-controller;
#gpio-cells = <2>;
--
2.35.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 0/3] arm64: dts: hi3798cv200: fix GICR size, add cache info, maintenance irq and GICH, GICV spaces
From: Yang Xiwen @ 2024-04-08 9:09 UTC (permalink / raw)
To: Krzysztof Kozlowski, Wei Xu, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Alex Elder, Peter Griffin
Cc: linux-arm-kernel, devicetree, linux-kernel, stable
In-Reply-To: <171256140981.12523.5652563259173425537.b4-ty@linaro.org>
On 4/8/2024 3:31 PM, Krzysztof Kozlowski wrote:
> On Mon, 19 Feb 2024 23:05:25 +0800, Yang Xiwen wrote:
>> The patchset fixes some warnings reported by the kernel during boot.
>>
>> The cache size info is from Processor_Datasheet_v2XX.pdf [1], Section
>> 2.2.1 Master Processor.
>>
>> The cache line size and the set-associative info are from Cortex-A53
>> Documentation [2].
>>
>> [...]
> It's rc3 and almost one month after last ping/talk, so apparently these got
> lost. I'll take them, but let me know if this should go via different tree.
Thanks a lot. From my experience, i think this should go via HiSilicon's
tree first(which stalls now), then go to SOC tree
(git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git), finally in
torvald's tree. This was the case for some qcom dts changes about 1yr ago.
>
>
> Applied, thanks!
>
> [1/3] arm64: dts: hi3798cv200: fix the size of GICR
> https://git.kernel.org/krzk/linux-dt/c/428a575dc9038846ad259466d5ba109858c0a023
> [2/3] arm64: dts: hi3798cv200: add GICH, GICV register space and irq
> https://git.kernel.org/krzk/linux-dt/c/f00a6b9644a5668e25ad9ca5aff53b6de4b0aaf6
> [3/3] arm64: dts: hi3798cv200: add cache info
> https://git.kernel.org/krzk/linux-dt/c/c7a3ad884d1dc1302dcc3295baa18917180b8bec
>
> Best regards,
--
Regards,
Yang Xiwen
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code
From: Arseniy Krasnov @ 2024-04-08 8:59 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Kevin Hilman, Jerome Brunet, Martin Blumenstingl
Cc: linux-mtd, devicetree, linux-arm-kernel, linux-amlogic,
linux-kernel, oxffffaa, kernel, Arseniy Krasnov
In-Reply-To: <20240408085931.456337-1-avkrasnov@salutedevices.com>
Boot ROM code on Meson requires that some pages on NAND must be written
in special mode: "short" ECC mode where each block is 384 bytes and
scrambling mode is on. Such pages located with the specified interval
within specified offset. Both interval and offset are located in the
device tree and used by driver if 'nand-is-boot-medium' is set for
NAND chip.
Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
---
.../bindings/mtd/amlogic,meson-nand.yaml | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
index 57b6957c8415..80ba5003ca70 100644
--- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
@@ -64,11 +64,25 @@ patternProperties:
items:
maximum: 0
+ amlogic,boot-page-last:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ The NFC driver needs this information to select ECC
+ algorithms supported by the boot ROM.
+
+ amlogic,boot-page-step:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ The NFC driver needs this information to select ECC
+ algorithms supported by the boot ROM (in pages).
+
unevaluatedProperties: false
dependencies:
nand-ecc-strength: [nand-ecc-step-size]
nand-ecc-step-size: [nand-ecc-strength]
+ amlogic,boot-page-last: [nand-is-boot-medium, amlogic,boot-page-step]
+ amlogic,boot-page-step: [nand-is-boot-medium, amlogic,boot-page-last]
required:
--
2.35.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/2] mtd: rawnand: meson: support R/W mode for boot ROM
From: Arseniy Krasnov @ 2024-04-08 8:59 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Kevin Hilman, Jerome Brunet, Martin Blumenstingl
Cc: linux-mtd, devicetree, linux-arm-kernel, linux-amlogic,
linux-kernel, oxffffaa, kernel, Arseniy Krasnov
In-Reply-To: <20240408085931.456337-1-avkrasnov@salutedevices.com>
Boot ROM code on Meson requires that some pages on NAND must be written
in special mode: "short" ECC mode where each block is 384 bytes and
scrambling mode is on. Such pages located with the specified interval
within specified offset. Both interval and offset are located in the
device tree and used by driver if 'nand-is-boot-medium' is set for
NAND chip.
Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
---
drivers/mtd/nand/raw/meson_nand.c | 88 +++++++++++++++++++++----------
1 file changed, 59 insertions(+), 29 deletions(-)
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index 00ce0e5bb970..b68aac24af16 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -35,6 +35,7 @@
#define NFC_CMD_RB BIT(20)
#define NFC_CMD_SCRAMBLER_ENABLE BIT(19)
#define NFC_CMD_SCRAMBLER_DISABLE 0
+#define NFC_CMD_SHORTMODE_ENABLE 1
#define NFC_CMD_SHORTMODE_DISABLE 0
#define NFC_CMD_RB_INT BIT(14)
#define NFC_CMD_RB_INT_NO_PIN ((0xb << 10) | BIT(18) | BIT(16))
@@ -78,6 +79,8 @@
#define DMA_DIR(dir) ((dir) ? NFC_CMD_N2M : NFC_CMD_M2N)
#define DMA_ADDR_ALIGN 8
+#define NFC_SHORT_MODE_ECC_SZ 384
+
#define ECC_CHECK_RETURN_FF (-1)
#define NAND_CE0 (0xe << 10)
@@ -125,6 +128,8 @@ struct meson_nfc_nand_chip {
u32 twb;
u32 tadl;
u32 tbers_max;
+ u32 boot_page_last;
+ u32 boot_page_step;
u32 bch_mode;
u8 *data_buf;
@@ -298,28 +303,49 @@ static void meson_nfc_cmd_seed(struct meson_nfc *nfc, u32 seed)
nfc->reg_base + NFC_REG_CMD);
}
-static void meson_nfc_cmd_access(struct nand_chip *nand, int raw, bool dir,
- int scrambler)
+static int meson_nfc_page_is_boot(struct nand_chip *nand, int page)
+{
+ const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
+
+ return (nand->options & NAND_IS_BOOT_MEDIUM) &&
+ !(page % meson_chip->boot_page_step) &&
+ (page < meson_chip->boot_page_last);
+}
+
+static void meson_nfc_cmd_access(struct nand_chip *nand, bool raw, bool dir, int page)
{
+ const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
struct mtd_info *mtd = nand_to_mtd(nand);
struct meson_nfc *nfc = nand_get_controller_data(mtd_to_nand(mtd));
- struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
- u32 bch = meson_chip->bch_mode, cmd;
int len = mtd->writesize, pagesize, pages;
+ int scrambler;
+ u32 cmd;
- pagesize = nand->ecc.size;
+ if (nand->options & NAND_NEED_SCRAMBLING)
+ scrambler = NFC_CMD_SCRAMBLER_ENABLE;
+ else
+ scrambler = NFC_CMD_SCRAMBLER_DISABLE;
if (raw) {
len = mtd->writesize + mtd->oobsize;
cmd = len | scrambler | DMA_DIR(dir);
- writel(cmd, nfc->reg_base + NFC_REG_CMD);
- return;
- }
+ } else if (meson_nfc_page_is_boot(nand, page)) {
+ pagesize = NFC_SHORT_MODE_ECC_SZ >> 3;
+ pages = mtd->writesize / 512;
+
+ scrambler = NFC_CMD_SCRAMBLER_ENABLE;
+ cmd = CMDRWGEN(DMA_DIR(dir), scrambler, NFC_ECC_BCH8_1K,
+ NFC_CMD_SHORTMODE_ENABLE, pagesize, pages);
+ } else {
+ pagesize = nand->ecc.size >> 3;
+ pages = len / nand->ecc.size;
- pages = len / nand->ecc.size;
+ cmd = CMDRWGEN(DMA_DIR(dir), scrambler, meson_chip->bch_mode,
+ NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
+ }
- cmd = CMDRWGEN(DMA_DIR(dir), scrambler, bch,
- NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
+ if (scrambler == NFC_CMD_SCRAMBLER_ENABLE)
+ meson_nfc_cmd_seed(nfc, page);
writel(cmd, nfc->reg_base + NFC_REG_CMD);
}
@@ -743,15 +769,7 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
if (ret)
return ret;
- if (nand->options & NAND_NEED_SCRAMBLING) {
- meson_nfc_cmd_seed(nfc, page);
- meson_nfc_cmd_access(nand, raw, DIRWRITE,
- NFC_CMD_SCRAMBLER_ENABLE);
- } else {
- meson_nfc_cmd_access(nand, raw, DIRWRITE,
- NFC_CMD_SCRAMBLER_DISABLE);
- }
-
+ meson_nfc_cmd_access(nand, raw, DIRWRITE, page);
cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
writel(cmd, nfc->reg_base + NFC_REG_CMD);
meson_nfc_queue_rb(nand, PSEC_TO_MSEC(sdr->tPROG_max), false);
@@ -829,15 +847,7 @@ static int meson_nfc_read_page_sub(struct nand_chip *nand,
if (ret)
return ret;
- if (nand->options & NAND_NEED_SCRAMBLING) {
- meson_nfc_cmd_seed(nfc, page);
- meson_nfc_cmd_access(nand, raw, DIRREAD,
- NFC_CMD_SCRAMBLER_ENABLE);
- } else {
- meson_nfc_cmd_access(nand, raw, DIRREAD,
- NFC_CMD_SCRAMBLER_DISABLE);
- }
-
+ meson_nfc_cmd_access(nand, raw, DIRREAD, page);
ret = meson_nfc_wait_dma_finish(nfc);
meson_nfc_check_ecc_pages_valid(nfc, nand, raw);
@@ -1436,6 +1446,26 @@ meson_nfc_nand_chip_init(struct device *dev,
if (ret)
return ret;
+ if (nand->options & NAND_IS_BOOT_MEDIUM) {
+ ret = of_property_read_u32(np, "amlogic,boot-page-last",
+ &meson_chip->boot_page_last);
+ if (ret) {
+ dev_err(dev, "could not retrieve 'amlogic,boot-page-last' property: %d",
+ ret);
+ nand_cleanup(nand);
+ return ret;
+ }
+
+ ret = of_property_read_u32(np, "amlogic,boot-page-step",
+ &meson_chip->boot_page_step);
+ if (ret) {
+ dev_err(dev, "could not retrieve 'amlogic,boot-page-step' property: %d",
+ ret);
+ nand_cleanup(nand);
+ return ret;
+ }
+ }
+
ret = mtd_device_register(mtd, NULL, 0);
if (ret) {
dev_err(dev, "failed to register MTD device: %d\n", ret);
--
2.35.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/2] Meson: R/W support for pages used by boot ROM
From: Arseniy Krasnov @ 2024-04-08 8:59 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Kevin Hilman, Jerome Brunet, Martin Blumenstingl
Cc: linux-mtd, devicetree, linux-arm-kernel, linux-amlogic,
linux-kernel, oxffffaa, kernel, Arseniy Krasnov
Amlogic's boot ROM code needs that some pages on NAND must be written
in special "short" ECC mode with scrambling enabled. Such pages:
1) Contain some metadata about hardware.
2) Located with some interval starting from 0 offset, until some
specified offset. Interval and second offset are set in the
device tree.
This patchset adds R/W support for such pages. To enable it we can setup
it in dts:
nand-is-boot-medium;
amlogic,boot-page-last = <1024>;
amlogic,boot-page-step = <128>;
It means that each 128th page in range 0 to 1024 pages will be accessed
in special mode ("short" ECC + scrambling). In practice this feature is
needed when we want to update first block of NAND - driver will enable
required mode by itself using value from device tree.
Changelog:
v1 -> v2:
* Rename 'meson,boot-page-XXX' -> 'amlogic,boot-page-XXX'.
* Add words that 'amlogic,boot-page-step' is measured in pages.
* Remove words that 'amlogic,boot-page-XXX' depends on 'nand-is-boot-medium'.
* Make both 'amlogic,boot-page-XXX' depend on each other also, in
addition to 'nand-is-boot-medium' dependency.
Arseniy Krasnov (2):
dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code
mtd: rawnand: meson: support R/W mode for boot ROM
.../bindings/mtd/amlogic,meson-nand.yaml | 14 +++
drivers/mtd/nand/raw/meson_nand.c | 88 +++++++++++++------
2 files changed, 73 insertions(+), 29 deletions(-)
--
2.35.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 53/64] i2c: st: reword according to newest specification
From: Wolfram Sang @ 2024-04-08 9:14 UTC (permalink / raw)
To: Easwar Hariharan
Cc: linux-i2c, Patrice Chotard, Andi Shyti, linux-arm-kernel,
linux-kernel
In-Reply-To: <f94f293c-4756-458e-8ffa-a10baa8379d0@linux.microsoft.com>
[-- Attachment #1.1: Type: text/plain, Size: 970 bytes --]
> > - unsigned long timeout;
> > + unsigned long time_left;
>
> Thanks for doing this. Is the timeout v/s time_left language also due to the specification change?
> A link to the specification(s) in the commit message would be nice to have
I admit it is probably a seperate change...
> > - if (!timeout) {
> > - dev_err(i2c_dev->dev, "Write to slave 0x%x timed out\n",
> > - c->addr);
... motivated by this "if (!timeout) dev_err("timeout!")" which is super
confusing to read because the logic is paradox.
> > + if (!time_left)
> > ret = -ETIMEDOUT;
> > - }
>
> Why did we lost the dev_err() here?
Agreed. Another seperate change. A timeout is not something the user
need to be aware of. It can regularly happen while an EEPROM is erasing
a page. The client driver will probably handle it correctly by trying
again. Only if the client driver sees a problem, then the user should be
notified. But not in the controller driver.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/2] thermal/drivers/mediatek/lvts_thermal: Improve some memory allocation
From: Dan Carpenter @ 2024-04-08 8:09 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Matthias Brugger, AngeloGioacchino Del Regno, linux-kernel,
kernel-janitors, linux-pm, linux-arm-kernel, linux-mediatek
In-Reply-To: <8cb69f245311a348164b0b5ca3dbc59386746035.1712520052.git.christophe.jaillet@wanadoo.fr>
On Sun, Apr 07, 2024 at 10:01:49PM +0200, Christophe JAILLET wrote:
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 3003dc350766..b133f731c5ba 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -204,7 +204,7 @@ static const struct debugfs_reg32 lvts_regs[] = {
>
> static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
> {
> - struct debugfs_regset32 *regset;
> + struct debugfs_regset32 *regsets;
> struct lvts_ctrl *lvts_ctrl;
> struct dentry *dentry;
> char name[64];
> @@ -214,8 +214,14 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
> if (IS_ERR(lvts_td->dom_dentry))
> return 0;
>
> + regsets = devm_kcalloc(dev, lvts_td->num_lvts_ctrl,
> + sizeof(*regsets), GFP_KERNEL);
> + if (!regsets)
> + return 0;
I understand that this preserved the behavior from the original code,
but the original code was wrong. This should return -ENOMEM.
> +
> for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {
>
> + struct debugfs_regset32 *regset = ®sets[i];
> lvts_ctrl = &lvts_td->lvts_ctrl[i];
The blank line should come after the declaration.
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] clk: meson: fix module license to GPL only
From: Neil Armstrong @ 2024-04-08 9:17 UTC (permalink / raw)
To: Jerome Brunet, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl
Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel,
Neil Armstrong
Fix the checkpatch warning:
WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
[1] https://lore.kernel.org/all/20240403-amlogic-v6-4-upstream-dsi-ccf-vim3-v12-0-99ecdfdc87fc@linaro.org/
---
drivers/clk/meson/axg-aoclk.c | 2 +-
drivers/clk/meson/axg-audio.c | 2 +-
drivers/clk/meson/axg.c | 2 +-
drivers/clk/meson/clk-cpu-dyndiv.c | 2 +-
drivers/clk/meson/clk-dualdiv.c | 2 +-
drivers/clk/meson/clk-mpll.c | 2 +-
drivers/clk/meson/clk-phase.c | 2 +-
drivers/clk/meson/clk-pll.c | 2 +-
drivers/clk/meson/clk-regmap.c | 2 +-
drivers/clk/meson/g12a-aoclk.c | 2 +-
drivers/clk/meson/g12a.c | 2 +-
drivers/clk/meson/gxbb-aoclk.c | 2 +-
drivers/clk/meson/gxbb.c | 2 +-
drivers/clk/meson/meson-aoclk.c | 2 +-
drivers/clk/meson/meson-eeclk.c | 2 +-
drivers/clk/meson/sclk-div.c | 2 +-
drivers/clk/meson/vclk.c | 2 +-
drivers/clk/meson/vid-pll-div.c | 2 +-
18 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
index d80ab4728f7a..e4d0f46f47f5 100644
--- a/drivers/clk/meson/axg-aoclk.c
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -340,4 +340,4 @@ static struct platform_driver axg_aoclkc_driver = {
};
module_platform_driver(axg_aoclkc_driver);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
index ac3482960903..e03a5bf899c0 100644
--- a/drivers/clk/meson/axg-audio.c
+++ b/drivers/clk/meson/axg-audio.c
@@ -1877,4 +1877,4 @@ module_platform_driver(axg_audio_driver);
MODULE_DESCRIPTION("Amlogic AXG/G12A/SM1 Audio Clock driver");
MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 5f60f2bcca59..52d610110e44 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -2185,4 +2185,4 @@ static struct platform_driver axg_driver = {
};
module_platform_driver(axg_driver);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/clk-cpu-dyndiv.c b/drivers/clk/meson/clk-cpu-dyndiv.c
index 8778c149d26a..aa824b030cb8 100644
--- a/drivers/clk/meson/clk-cpu-dyndiv.c
+++ b/drivers/clk/meson/clk-cpu-dyndiv.c
@@ -69,4 +69,4 @@ EXPORT_SYMBOL_GPL(meson_clk_cpu_dyndiv_ops);
MODULE_DESCRIPTION("Amlogic CPU Dynamic Clock divider");
MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/clk-dualdiv.c b/drivers/clk/meson/clk-dualdiv.c
index feae49a8f6dc..d46c02b51be5 100644
--- a/drivers/clk/meson/clk-dualdiv.c
+++ b/drivers/clk/meson/clk-dualdiv.c
@@ -140,4 +140,4 @@ EXPORT_SYMBOL_GPL(meson_clk_dualdiv_ro_ops);
MODULE_DESCRIPTION("Amlogic dual divider driver");
MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c
index 20255e129b37..eae9b7dc5a6c 100644
--- a/drivers/clk/meson/clk-mpll.c
+++ b/drivers/clk/meson/clk-mpll.c
@@ -177,4 +177,4 @@ EXPORT_SYMBOL_GPL(meson_clk_mpll_ops);
MODULE_DESCRIPTION("Amlogic MPLL driver");
MODULE_AUTHOR("Michael Turquette <mturquette@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c
index a6763439f7d2..ff3f0b1a3ed1 100644
--- a/drivers/clk/meson/clk-phase.c
+++ b/drivers/clk/meson/clk-phase.c
@@ -183,4 +183,4 @@ EXPORT_SYMBOL_GPL(meson_sclk_ws_inv_ops);
MODULE_DESCRIPTION("Amlogic phase driver");
MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 6fa7639a3050..7f96ede7fdbe 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -486,4 +486,4 @@ EXPORT_SYMBOL_GPL(meson_clk_pll_ro_ops);
MODULE_DESCRIPTION("Amlogic PLL driver");
MODULE_AUTHOR("Carlo Caione <carlo@endlessm.com>");
MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/clk-regmap.c b/drivers/clk/meson/clk-regmap.c
index 8ad8977cf1c2..ad116d24f700 100644
--- a/drivers/clk/meson/clk-regmap.c
+++ b/drivers/clk/meson/clk-regmap.c
@@ -183,4 +183,4 @@ EXPORT_SYMBOL_GPL(clk_regmap_mux_ro_ops);
MODULE_DESCRIPTION("Amlogic regmap backed clock driver");
MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
index c6b1d55cd7c8..58976ed8b92a 100644
--- a/drivers/clk/meson/g12a-aoclk.c
+++ b/drivers/clk/meson/g12a-aoclk.c
@@ -475,4 +475,4 @@ static struct platform_driver g12a_aoclkc_driver = {
};
module_platform_driver(g12a_aoclkc_driver);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 083882e53b65..c7c3a9259427 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -5610,4 +5610,4 @@ static struct platform_driver g12a_driver = {
};
module_platform_driver(g12a_driver);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 4aec1740ac34..dbda563729db 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -300,4 +300,4 @@ static struct platform_driver gxbb_aoclkc_driver = {
},
};
module_platform_driver(gxbb_aoclkc_driver);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index 1b1279d94781..29507b8c4304 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -3569,4 +3569,4 @@ static struct platform_driver gxbb_driver = {
};
module_platform_driver(gxbb_driver);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/meson-aoclk.c b/drivers/clk/meson/meson-aoclk.c
index bf466fef263c..b8a9d59e6726 100644
--- a/drivers/clk/meson/meson-aoclk.c
+++ b/drivers/clk/meson/meson-aoclk.c
@@ -89,4 +89,4 @@ int meson_aoclkc_probe(struct platform_device *pdev)
return devm_of_clk_add_hw_provider(dev, meson_clk_hw_get, (void *)&data->hw_clks);
}
EXPORT_SYMBOL_GPL(meson_aoclkc_probe);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/meson-eeclk.c b/drivers/clk/meson/meson-eeclk.c
index 845ca8bfa346..3cbc7f233bba 100644
--- a/drivers/clk/meson/meson-eeclk.c
+++ b/drivers/clk/meson/meson-eeclk.c
@@ -58,4 +58,4 @@ int meson_eeclkc_probe(struct platform_device *pdev)
return devm_of_clk_add_hw_provider(dev, meson_clk_hw_get, (void *)&data->hw_clks);
}
EXPORT_SYMBOL_GPL(meson_eeclkc_probe);
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
index d12c45c4c261..987f5b06587c 100644
--- a/drivers/clk/meson/sclk-div.c
+++ b/drivers/clk/meson/sclk-div.c
@@ -251,4 +251,4 @@ EXPORT_SYMBOL_GPL(meson_sclk_div_ops);
MODULE_DESCRIPTION("Amlogic Sample divider driver");
MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
index 3ea813a0a995..c0a1d24844bb 100644
--- a/drivers/clk/meson/vclk.c
+++ b/drivers/clk/meson/vclk.c
@@ -138,4 +138,4 @@ EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
MODULE_DESCRIPTION("Amlogic vclk clock driver");
MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/vid-pll-div.c b/drivers/clk/meson/vid-pll-div.c
index daff235bc763..ee129f86794d 100644
--- a/drivers/clk/meson/vid-pll-div.c
+++ b/drivers/clk/meson/vid-pll-div.c
@@ -96,4 +96,4 @@ EXPORT_SYMBOL_GPL(meson_vid_pll_div_ro_ops);
MODULE_DESCRIPTION("Amlogic video pll divider driver");
MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
---
base-commit: a3c4ce7a4b9c1fe78f66afeea6eb89c68ef45f9c
change-id: 20240408-amlogic-v6-9-upstream-fix-clk-module-license-34060f9b5192
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v12 2/7] clk: meson: add vclk driver
From: neil.armstrong @ 2024-04-08 9:19 UTC (permalink / raw)
To: Jerome Brunet
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Martin Blumenstingl, Kevin Hilman, Michael Turquette,
Stephen Boyd, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Jagan Teki, Nicolas Belin,
devicetree, linux-kernel, linux-amlogic, linux-clk,
linux-arm-kernel, dri-devel
In-Reply-To: <1ja5m8p4n4.fsf@starbuckisacylon.baylibre.com>
On 05/04/2024 09:00, Jerome Brunet wrote:
>
> On Thu 04 Apr 2024 at 18:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
>> On 04/04/2024 10:13, Jerome Brunet wrote:
>>> On Wed 03 Apr 2024 at 09:46, Neil Armstrong <neil.armstrong@linaro.org>
>>> wrote:
>>>
>>>> The VCLK and VCLK_DIV clocks have supplementary bits.
>>>>
>>>> The VCLK gate has a "SOFT RESET" bit to toggle after the whole
>>>> VCLK sub-tree rate has been set, this is implemented in
>>>> the gate enable callback.
>>>>
>>>> The VCLK_DIV clocks as enable and reset bits used to disable
>>>> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
>>>> the rate is set while the divider is disabled and in reset mode.
>>>>
>>>> The VCLK_DIV enable bit isn't implemented as a gate since it's part
>>>> of the divider logic and vendor does this exact sequence to ensure
>>>> the divider is correctly set.
>>> The checkpatch warning is still there. Is it a choice or a mistake ?
>>> Documentation says "GPL v2" exists for historic reason which seems to
>>> hint "GPL" is preferred, and I suppose this is why checkpatch warns for
>>> it.
>>
>> Well I didn't see this warning, this is what I fixed:
>>
>> $ scripts/checkpatch.pl --strict drivers/clk/meson/vclk.c
>> CHECK: Alignment should match open parenthesis
>> #63: FILE: drivers/clk/meson/vclk.c:63:
>> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
>> + unsigned long prate)
>>
>> CHECK: Alignment should match open parenthesis
>> #73: FILE: drivers/clk/meson/vclk.c:73:
>> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
>> + struct clk_rate_request *req)
>>
>> CHECK: Alignment should match open parenthesis
>> #83: FILE: drivers/clk/meson/vclk.c:83:
>> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long parent_rate)
>>
>
> I would not ask a respin solely for this. It's nice to fix it but I was
> mostly after the warning TBH.
>
>> <snip>
>>
>> It seems that checking a commit triggers an extra check....
>>
>> $ scripts/checkpatch.pl --strict -G 1bac9f6aa3c3
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>> #58:
>> new file mode 100644
>>
>> <snip>
>>
>> WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
>> #203: FILE: drivers/clk/meson/vclk.c:141:
>> +MODULE_LICENSE("GPL v2");
>
> Hum, I'm running checkpatch against the mail itself, not the commit. I
> still get the warning
Patch or commit seems to trigger more tests than a file directly, anyway I sent a follow-up patch:
https://lore.kernel.org/all/20240408-amlogic-v6-9-upstream-fix-clk-module-license-v1-1-366ddc0f3db9@linaro.org/
Thanks,
Neil
>
>>
>> <snip>
>>
>> Neil
>>
>>>
>>>>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>> drivers/clk/meson/Kconfig | 4 ++
>>>> drivers/clk/meson/Makefile | 1 +
>>>> drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
>>>> drivers/clk/meson/vclk.h | 51 ++++++++++++++++
>>>> 4 files changed, 197 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>> index 29ffd14d267b..8a9823789fa3 100644
>>>> --- a/drivers/clk/meson/Kconfig
>>>> +++ b/drivers/clk/meson/Kconfig
>>>> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>>>> tristate
>>>> select COMMON_CLK_MESON_REGMAP
>>>> +config COMMON_CLK_MESON_VCLK
>>>> + tristate
>>>> + select COMMON_CLK_MESON_REGMAP
>>>> +
>>>> config COMMON_CLK_MESON_CLKC_UTILS
>>>> tristate
>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>> index 9ee4b954c896..9ba43fe7a07a 100644
>>>> --- a/drivers/clk/meson/Makefile
>>>> +++ b/drivers/clk/meson/Makefile
>>>> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>>>> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>>>> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>>>> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
>>>> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>>>> # Amlogic Clock controllers
>>>> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
>>>> new file mode 100644
>>>> index 000000000000..45dc216941ea
>>>> --- /dev/null
>>>> +++ b/drivers/clk/meson/vclk.c
>>>> @@ -0,0 +1,141 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (c) 2024 Neil Armstrong <neil.armstrong@linaro.org>
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include "vclk.h"
>>>> +
>>>> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
>>>> +
>>>> +static inline struct meson_vclk_gate_data *
>>>> +clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
>>>> +{
>>>> + return (struct meson_vclk_gate_data *)clk->data;
>>>> +}
>>>> +
>>>> +static int meson_vclk_gate_enable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>>> +
>>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>>> +
>>>> + /* Do a reset pulse */
>>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void meson_vclk_gate_disable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>>> +
>>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>>> +}
>>>> +
>>>> +static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>>> +
>>>> + return meson_parm_read(clk->map, &vclk->enable);
>>>> +}
>>>> +
>>>> +const struct clk_ops meson_vclk_gate_ops = {
>>>> + .enable = meson_vclk_gate_enable,
>>>> + .disable = meson_vclk_gate_disable,
>>>> + .is_enabled = meson_vclk_gate_is_enabled,
>>>> +};
>>>> +EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
>>>> +
>>>> +/* The VCLK Divider has supplementary reset & enable bits */
>>>> +
>>>> +static inline struct meson_vclk_div_data *
>>>> +clk_get_meson_vclk_div_data(struct clk_regmap *clk)
>>>> +{
>>>> + return (struct meson_vclk_div_data *)clk->data;
>>>> +}
>>>> +
>>>> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
>>>> + unsigned long prate)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
>>>> + vclk->table, vclk->flags, vclk->div.width);
>>>> +}
>>>> +
>>>> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
>>>> + struct clk_rate_request *req)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
>>>> + vclk->flags);
>>>> +}
>>>> +
>>>> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>>>> + unsigned long parent_rate)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> + int ret;
>>>> +
>>>> + ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
>>>> + vclk->flags);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + meson_parm_write(clk->map, &vclk->div, ret);
>>>> +
>>>> + return 0;
>>>> +};
>>>> +
>>>> +static int meson_vclk_div_enable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + /* Unreset the divider when ungating */
>>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void meson_vclk_div_disable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + /* Reset the divider when gating */
>>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>>> +}
>>>> +
>>>> +static int meson_vclk_div_is_enabled(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + return meson_parm_read(clk->map, &vclk->enable);
>>>> +}
>>>> +
>>>> +const struct clk_ops meson_vclk_div_ops = {
>>>> + .recalc_rate = meson_vclk_div_recalc_rate,
>>>> + .determine_rate = meson_vclk_div_determine_rate,
>>>> + .set_rate = meson_vclk_div_set_rate,
>>>> + .enable = meson_vclk_div_enable,
>>>> + .disable = meson_vclk_div_disable,
>>>> + .is_enabled = meson_vclk_div_is_enabled,
>>>> +};
>>>> +EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
>>>> +
>>>> +MODULE_DESCRIPTION("Amlogic vclk clock driver");
>>>> +MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
>>>> +MODULE_LICENSE("GPL v2");
>>>> diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
>>>> new file mode 100644
>>>> index 000000000000..20b0b181db09
>>>> --- /dev/null
>>>> +++ b/drivers/clk/meson/vclk.h
>>>> @@ -0,0 +1,51 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +/*
>>>> + * Copyright (c) 2024 Neil Armstrong <neil.armstrong@linaro.org>
>>>> + */
>>>> +
>>>> +#ifndef __VCLK_H
>>>> +#define __VCLK_H
>>>> +
>>>> +#include "clk-regmap.h"
>>>> +#include "parm.h"
>>>> +
>>>> +/**
>>>> + * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
>>>> + *
>>>> + * @enable: vclk enable field
>>>> + * @reset: vclk reset field
>>>> + * @flags: hardware-specific flags
>>>> + *
>>>> + * Flags:
>>>> + * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
>>>> + */
>>>> +struct meson_vclk_gate_data {
>>>> + struct parm enable;
>>>> + struct parm reset;
>>>> + u8 flags;
>>>> +};
>>>> +
>>>> +extern const struct clk_ops meson_vclk_gate_ops;
>>>> +
>>>> +/**
>>>> + * struct meson_vclk_div_data - vclk_div regmap back specific data
>>>> + *
>>>> + * @div: divider field
>>>> + * @enable: vclk divider enable field
>>>> + * @reset: vclk divider reset field
>>>> + * @table: array of value/divider pairs, last entry should have div = 0
>>>> + *
>>>> + * Flags:
>>>> + * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
>>>> + */
>>>> +struct meson_vclk_div_data {
>>>> + struct parm div;
>>>> + struct parm enable;
>>>> + struct parm reset;
>>>> + const struct clk_div_table *table;
>>>> + u8 flags;
>>>> +};
>>>> +
>>>> +extern const struct clk_ops meson_vclk_div_ops;
>>>> +
>>>> +#endif /* __VCLK_H */
>>>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Ian Ray @ 2024-04-08 9:24 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Ian Ray, devicetree, imx, linux-arm-kernel, linux-kernel
Add i2c{1,6} sda-/scl-gpios with the corresponding pinmux entries.
Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
.../boot/dts/freescale/imx8mp-msc-sm2s.dtsi | 22 +++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
index 61c2a63efc6d..7e4327084d26 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
@@ -200,8 +200,11 @@ ethphy1: ethernet-phy@1 {
};
&i2c1 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c1>;
+ pinctrl-1 = <&pinctrl_i2c1_gpio>;
+ scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
clock-frequency = <400000>;
status = "okay";
@@ -241,8 +244,11 @@ &i2c5 {
};
&i2c6 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c6>;
+ pinctrl-1 = <&pinctrl_i2c6_gpio>;
+ scl-gpios = <&gpio3 19 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ sda-gpios = <&gpio3 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
clock-frequency = <400000>;
status = "okay";
@@ -606,6 +612,12 @@ pinctrl_i2c1: i2c1grp {
<MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA 0x400001c3>;
};
+ pinctrl_i2c1_gpio: i2c1gpiogrp {
+ fsl,pins =
+ <MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14 0x400001c3>,
+ <MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15 0x400001c3>;
+ };
+
pinctrl_i2c2: i2c2grp {
fsl,pins =
<MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c3>,
@@ -636,6 +648,12 @@ pinctrl_i2c6: i2c6grp {
<MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA 0x400001c3>;
};
+ pinctrl_i2c6_gpio: i2c5gpiogrp {
+ fsl,pins =
+ <MX8MP_IOMUXC_SAI5_RXFS__GPIO3_IO19 0x400001c3>,
+ <MX8MP_IOMUXC_SAI5_RXC__GPIO3_IO20 0x400001c3>;
+ };
+
pinctrl_lcd0_backlight: lcd0-backlightgrp {
fsl,pins =
<MX8MP_IOMUXC_GPIO1_IO05__GPIO1_IO05 0x41>;
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/2] firmware: arm_scmi: Add dedicated vendor protocols submenu
From: Cristian Marussi @ 2024-04-08 9:30 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, vincent.guittot,
peng.fan, michal.simek, quic_sibis, konrad.dybcio,
souvik.chakravarty, Cristian Marussi
In-Reply-To: <20240408093052.3801576-1-cristian.marussi@arm.com>
Add a dedicated Kconfig submenu and directory where to collect SCMI vendor
protocols implementations.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/Kconfig | 2 ++
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/vendors/Kconfig | 4 ++++
drivers/firmware/arm_scmi/vendors/Makefile | 2 ++
4 files changed, 9 insertions(+)
create mode 100644 drivers/firmware/arm_scmi/vendors/Kconfig
create mode 100644 drivers/firmware/arm_scmi/vendors/Makefile
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index aa5842be19b2..db9166e99177 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -180,4 +180,6 @@ config ARM_SCMI_POWER_CONTROL
called scmi_power_control. Note this may needed early in boot to catch
early shutdown/reboot SCMI requests.
+source "drivers/firmware/arm_scmi/vendors/Kconfig"
+
endmenu
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index a7bc4796519c..1316892a230d 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -15,6 +15,7 @@ scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
+obj-$(CONFIG_ARM_SCMI_PROTOCOL) += vendors/
obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o
diff --git a/drivers/firmware/arm_scmi/vendors/Kconfig b/drivers/firmware/arm_scmi/vendors/Kconfig
new file mode 100644
index 000000000000..7c1ca7a12603
--- /dev/null
+++ b/drivers/firmware/arm_scmi/vendors/Kconfig
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "ARM SCMI Vendor Protocols"
+
+endmenu
diff --git a/drivers/firmware/arm_scmi/vendors/Makefile b/drivers/firmware/arm_scmi/vendors/Makefile
new file mode 100644
index 000000000000..c6c214158dd8
--- /dev/null
+++ b/drivers/firmware/arm_scmi/vendors/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# obj-$(CONFIG_ARM_SCMI_PROTOCOL_<your_vendor_proto>) += <your_vendor_proto>.o
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/2] SCMI multiple vendor protocol support
From: Cristian Marussi @ 2024-04-08 9:30 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, vincent.guittot,
peng.fan, michal.simek, quic_sibis, konrad.dybcio,
souvik.chakravarty, Cristian Marussi
Hi all,
this is meant to address the possibility of having multiple vendors
implementing distinct SCMI vendor protocols with possibly the same
overlapping protocol number without the need of crafting the Kconfig
at compile time to include only wanted protos
Basic idea is simple:
- vendor protos HAS to be 'tagged' at build time with a vendor_id
- vendor protos COULD also optionally be tagged at build time with
sub_vendor_id and implementation version
- at init all the build vendor protos are registered with the SCMI core
using a key derived from the above tags
- at SCMI probe time the platform is identified via Base protocol as
usual and all the required vendor protos (i.e. IDs defined in the DT
as usual) are loaded (if required) after a lookup process based on the
following rules:
+ protocol DB is searched using the platform/Base runtime provided tags
<vendor> / <sub_vendor> / <impl_ver>
using the following search logic (keys), first match:
1. proto_id / <vendor_id> / <sub_vendor_id> / <impl_ver>
2. proto_id / <vendor_id> / <sub_vendor_id> / 0
3. proto_id / <vendor_id> / NULL / 0
IOW, closest match, depending on how much fine grained is your
protocol tuned (tagged) for the platform.
I am still doubtful about the usage of <impl_ver>, and tempted to drop it
since we have anyway protocol version and NEGOTIATE_PROTOCOL_VERSION
to deal with slight difference in fw revision...
In V2 a dedicated vendors dir/Makefile/Kconfig is added: not sure if we
want to provide also a way of grouping SCMI vendor protocol headers under
include/linux...open to any suggestion/opinion.
Based on sudeep/for-next/scmi/updates on top of
commit dccc943c8953 ("clk: scmi: Support get/set duty_cycle operations")
Any feedback welcome
Thanks,
Cristian
---
v1 -> v2
- added a dedicated vendors/ directory and Kconfig under arm_scmi/
Cristian Marussi (2):
firmware: arm_scmi: Add support for multiple vendors custom protocols
firmware: arm_scmi: Add dedicated vendor protocols submenu
drivers/firmware/arm_scmi/Kconfig | 2 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/driver.c | 165 ++++++++++++++++++---
drivers/firmware/arm_scmi/protocols.h | 15 ++
drivers/firmware/arm_scmi/vendors/Kconfig | 4 +
drivers/firmware/arm_scmi/vendors/Makefile | 2 +
6 files changed, 167 insertions(+), 22 deletions(-)
create mode 100644 drivers/firmware/arm_scmi/vendors/Kconfig
create mode 100644 drivers/firmware/arm_scmi/vendors/Makefile
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/2] firmware: arm_scmi: Add support for multiple vendors custom protocols
From: Cristian Marussi @ 2024-04-08 9:30 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, vincent.guittot,
peng.fan, michal.simek, quic_sibis, konrad.dybcio,
souvik.chakravarty, Cristian Marussi
In-Reply-To: <20240408093052.3801576-1-cristian.marussi@arm.com>
Add a mechanism to be able to tag vendor protocol modules at compile-time
with a vendor/sub_vendor string and an implementation version and then to
choose to load, at run-time, only those vendor protocol modules matching
as close as possible the vendor/subvendor identification advertised by
the SCMI platform server.
In this way, any in-tree existent vendor protocol module can be build and
shipped by default in a single kernel image, even when using the same
clashing protocol identification numbers, since the SCMI core will take
care at run-time to load only the ones pertinent to the running system.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/driver.c | 165 ++++++++++++++++++++++----
drivers/firmware/arm_scmi/protocols.h | 15 +++
2 files changed, 158 insertions(+), 22 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index d0091459a276..aa18202054a5 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -33,6 +33,7 @@
#include <linux/processor.h>
#include <linux/refcount.h>
#include <linux/slab.h>
+#include <linux/xarray.h>
#include "common.h"
#include "notify.h"
@@ -44,8 +45,7 @@
static DEFINE_IDA(scmi_id);
-static DEFINE_IDR(scmi_protocols);
-static DEFINE_SPINLOCK(protocol_lock);
+static DEFINE_XARRAY(scmi_protocols);
/* List of all SCMI devices active in system */
static LIST_HEAD(scmi_list);
@@ -194,11 +194,90 @@ struct scmi_info {
#define bus_nb_to_scmi_info(nb) container_of(nb, struct scmi_info, bus_nb)
#define req_nb_to_scmi_info(nb) container_of(nb, struct scmi_info, dev_req_nb)
-static const struct scmi_protocol *scmi_protocol_get(int protocol_id)
+static unsigned long
+scmi_vendor_protocol_signature(unsigned int protocol_id, char *vendor_id,
+ char *sub_vendor_id, u32 impl_ver)
{
- const struct scmi_protocol *proto;
+ char *signature, *p;
+ unsigned long hash = 0;
- proto = idr_find(&scmi_protocols, protocol_id);
+ /* vendor_id/sub_vendor_id guaranteed <= SCMI_SHORT_NAME_MAX_SIZE */
+ signature = kasprintf(GFP_KERNEL, "%02X|%s|%s|0x%08X", protocol_id,
+ vendor_id ?: "", sub_vendor_id ?: "", impl_ver);
+ if (!signature)
+ return 0;
+
+ p = signature;
+ while (*p)
+ hash = partial_name_hash(tolower(*p++), hash);
+ hash = end_name_hash(hash);
+
+ kfree(signature);
+
+ return hash;
+}
+
+static unsigned long
+scmi_protocol_key_calculate(int protocol_id, char *vendor_id,
+ char *sub_vendor_id, u32 impl_ver)
+{
+ if (protocol_id < SCMI_PROTOCOL_VENDOR)
+ return protocol_id;
+ else
+ return scmi_vendor_protocol_signature(protocol_id, vendor_id,
+ sub_vendor_id, impl_ver);
+}
+
+static const struct scmi_protocol *
+scmi_vendor_protocol_lookup(int protocol_id, char *vendor_id,
+ char *sub_vendor_id, u32 impl_ver)
+{
+ unsigned long key;
+ struct scmi_protocol *proto = NULL;
+
+ /* Searching for closest match ...*/
+ key = scmi_protocol_key_calculate(protocol_id, vendor_id,
+ sub_vendor_id, impl_ver);
+ if (key)
+ proto = xa_load(&scmi_protocols, key);
+
+ if (proto)
+ return proto;
+
+ /* Any match on vendor/sub_vendor ? */
+ if (impl_ver) {
+ key = scmi_protocol_key_calculate(protocol_id, vendor_id,
+ sub_vendor_id, 0);
+ if (key)
+ proto = xa_load(&scmi_protocols, key);
+
+ if (proto)
+ return proto;
+ }
+
+ /* Any match on just the vendor ? */
+ if (sub_vendor_id) {
+ key = scmi_protocol_key_calculate(protocol_id, vendor_id,
+ NULL, 0);
+ if (key)
+ proto = xa_load(&scmi_protocols, key);
+ }
+
+ return proto;
+}
+
+static const struct scmi_protocol *
+scmi_protocol_get(int protocol_id, struct scmi_revision_info *version)
+{
+ const struct scmi_protocol *proto = NULL;
+
+ if (protocol_id < SCMI_PROTOCOL_VENDOR)
+ proto = xa_load(&scmi_protocols, protocol_id);
+ else
+ proto = scmi_vendor_protocol_lookup(protocol_id,
+ version->vendor_id,
+ version->sub_vendor_id,
+ version->impl_ver);
if (!proto || !try_module_get(proto->owner)) {
pr_warn("SCMI Protocol 0x%x not found!\n", protocol_id);
return NULL;
@@ -206,21 +285,46 @@ static const struct scmi_protocol *scmi_protocol_get(int protocol_id)
pr_debug("Found SCMI Protocol 0x%x\n", protocol_id);
+ if (protocol_id >= SCMI_PROTOCOL_VENDOR)
+ pr_info("Loaded SCMI Vendor Protocol 0x%x - %s %s %X\n",
+ protocol_id, proto->vendor_id ?: "",
+ proto->sub_vendor_id ?: "", proto->impl_ver);
+
return proto;
}
-static void scmi_protocol_put(int protocol_id)
+static void scmi_protocol_put(const struct scmi_protocol *proto)
{
- const struct scmi_protocol *proto;
-
- proto = idr_find(&scmi_protocols, protocol_id);
if (proto)
module_put(proto->owner);
}
+static int scmi_vendor_protocol_check(const struct scmi_protocol *proto)
+{
+ if (!proto->vendor_id) {
+ pr_err("missing vendor_id for protocol 0x%x\n", proto->id);
+ return -EINVAL;
+ }
+
+ if (strlen(proto->vendor_id) >= SCMI_SHORT_NAME_MAX_SIZE) {
+ pr_err("malformed vendor_id for protocol 0x%x\n", proto->id);
+ return -EINVAL;
+ }
+
+ if (proto->sub_vendor_id &&
+ strlen(proto->sub_vendor_id) >= SCMI_SHORT_NAME_MAX_SIZE) {
+ pr_err("malformed sub_vendor_id for protocol 0x%x\n",
+ proto->id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
int scmi_protocol_register(const struct scmi_protocol *proto)
{
int ret;
+ unsigned long key;
if (!proto) {
pr_err("invalid protocol\n");
@@ -232,12 +336,23 @@ int scmi_protocol_register(const struct scmi_protocol *proto)
return -EINVAL;
}
- spin_lock(&protocol_lock);
- ret = idr_alloc(&scmi_protocols, (void *)proto,
- proto->id, proto->id + 1, GFP_ATOMIC);
- spin_unlock(&protocol_lock);
- if (ret != proto->id) {
- pr_err("unable to allocate SCMI idr slot for 0x%x - err %d\n",
+ if (proto->id >= SCMI_PROTOCOL_VENDOR &&
+ scmi_vendor_protocol_check(proto))
+ return -EINVAL;
+
+ /*
+ * Calculate a protocol key to register this protocol with the core;
+ * key value 0 is considered invalid.
+ */
+ key = scmi_protocol_key_calculate(proto->id, proto->vendor_id,
+ proto->sub_vendor_id,
+ proto->impl_ver);
+ if (!key)
+ return -EINVAL;
+
+ ret = xa_insert(&scmi_protocols, key, (void *)proto, GFP_KERNEL);
+ if (ret) {
+ pr_err("unable to allocate SCMI protocol slot for 0x%x - err %d\n",
proto->id, ret);
return ret;
}
@@ -250,9 +365,15 @@ EXPORT_SYMBOL_GPL(scmi_protocol_register);
void scmi_protocol_unregister(const struct scmi_protocol *proto)
{
- spin_lock(&protocol_lock);
- idr_remove(&scmi_protocols, proto->id);
- spin_unlock(&protocol_lock);
+ unsigned long key;
+
+ key = scmi_protocol_key_calculate(proto->id, proto->vendor_id,
+ proto->sub_vendor_id,
+ proto->impl_ver);
+ if (!key)
+ return;
+
+ xa_erase(&scmi_protocols, key);
pr_debug("Unregistered SCMI Protocol 0x%x\n", proto->id);
}
@@ -1940,7 +2061,7 @@ scmi_alloc_init_protocol_instance(struct scmi_info *info,
/* Protocol specific devres group */
gid = devres_open_group(handle->dev, NULL, GFP_KERNEL);
if (!gid) {
- scmi_protocol_put(proto->id);
+ scmi_protocol_put(proto);
goto out;
}
@@ -2004,7 +2125,7 @@ scmi_alloc_init_protocol_instance(struct scmi_info *info,
clean:
/* Take care to put the protocol module's owner before releasing all */
- scmi_protocol_put(proto->id);
+ scmi_protocol_put(proto);
devres_release_group(handle->dev, gid);
out:
return ERR_PTR(ret);
@@ -2038,7 +2159,7 @@ scmi_get_protocol_instance(const struct scmi_handle *handle, u8 protocol_id)
const struct scmi_protocol *proto;
/* Fails if protocol not registered on bus */
- proto = scmi_protocol_get(protocol_id);
+ proto = scmi_protocol_get(protocol_id, &info->version);
if (proto)
pi = scmi_alloc_init_protocol_instance(info, proto);
else
@@ -2093,7 +2214,7 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id)
idr_remove(&info->protocols, protocol_id);
- scmi_protocol_put(protocol_id);
+ scmi_protocol_put(pi->proto);
devres_release_group(handle->dev, gid);
dev_dbg(handle->dev, "De-Initialized protocol: 0x%X\n",
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index 317d3fb32676..45ee8b35f56d 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -29,6 +29,8 @@
#define PROTOCOL_REV_MAJOR(x) ((u16)(FIELD_GET(PROTOCOL_REV_MAJOR_MASK, (x))))
#define PROTOCOL_REV_MINOR(x) ((u16)(FIELD_GET(PROTOCOL_REV_MINOR_MASK, (x))))
+#define SCMI_PROTOCOL_VENDOR 0x80
+
enum scmi_common_cmd {
PROTOCOL_VERSION = 0x0,
PROTOCOL_ATTRIBUTES = 0x1,
@@ -323,6 +325,16 @@ typedef int (*scmi_prot_init_ph_fn_t)(const struct scmi_protocol_handle *);
* protocol by the agent. Each protocol implementation
* in the agent is supposed to downgrade to match the
* protocol version supported by the platform.
+ * @vendor_id: A firmware vendor string for vendor protocols matching.
+ * Ignored when @id identifies a standard protocol, cannot be NULL
+ * otherwise.
+ * @sub_vendor_id: A firmware sub_vendor string for vendor protocols matching.
+ * Ignored if NULL or when @id identifies a standard protocol.
+ * @impl_ver: A firmware implementation version for vendor protocols matching.
+ * Ignored if zero or if @id identifies a standard protocol.
+ *
+ * Note that vendor protocols matching at load time is performed by attempting
+ * the closest match first against the tuple (vendor, sub_vendor, impl_ver)
*/
struct scmi_protocol {
const u8 id;
@@ -332,6 +344,9 @@ struct scmi_protocol {
const void *ops;
const struct scmi_protocol_events *events;
unsigned int supported_version;
+ char *vendor_id;
+ char *sub_vendor_id;
+ u32 impl_ver;
};
#define DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(name, proto) \
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 5/5] arm64: dts: ti: k3-am62*: Add PHY2 region to USB wrapper node
From: Francesco Dolcini @ 2024-04-08 9:52 UTC (permalink / raw)
To: Roger Quadros
Cc: nm, vigneshr, afd, kristo, robh+dt, krzysztof.kozlowski+dt,
conor+dt, srk, r-gunasekaran, b-liu, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240201120332.4811-6-rogerq@kernel.org>
Hello Roger,
On Thu, Feb 01, 2024 at 02:03:32PM +0200, Roger Quadros wrote:
> Add PHY2 register space to USB wrapper node. This is required
> to deal with Errata i2409.
>
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
What's the status/plan for this? v6.9-rc misses it and therefore we have
this error in the logs (and of course, we miss the workaround).
[ 0.583305] dwc3-am62 f910000.dwc3-usb: invalid resource (null)
[ 0.589304] dwc3-am62 f910000.dwc3-usb: can't map PHY IOMEM resource. Won't apply i2409 fix.
Apart for the error message, the change here seems required for the
hardware to properly function (IOW IMHO it should be back-ported to stable).
Francesco
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v2 1/5] clk: meson: axg: move reset controller's code to separate module
From: Stephen Boyd @ 2024-04-08 9:52 UTC (permalink / raw)
To: Jan Dakinevich, Jerome Brunet, Philipp Zabel
Cc: Neil Armstrong, Michael Turquette, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <f01cdd910ab35316b8012795f73fd2b34c8e6f8e.camel@pengutronix.de>
Quoting Philipp Zabel (2024-04-08 01:21:47)
> On So, 2024-04-07 at 19:39 -0700, Stephen Boyd wrote:
> > Quoting Jerome Brunet (2024-04-02 07:52:38)
> > >
> > > On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> > >
> > > > This code will by reused by A1 SoC.
> > >
> > > Could expand a bit please ?
> > >
> > > >
> > > > Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> > >
> > > In general, I like the idea.
> > >
> > > We do have a couple a reset registers lost in middle of clocks and this
> > > change makes it possible to re-use the code instead duplicating it.
> > >
> > > The exported function would be used by audio clock controllers, but the
> > > module created would be purely about reset.
> > >
> > > One may wonder how it ended up in the clock tree, especially since the
> > > kernel as a reset tree too.
> > >
> > > I'm not sure if this should move to the reset framework or if it would
> > > be an unnecessary churn. Stephen, Philipp, do you have an opinion on
> > > this ?
> > >
> >
> > I'd prefer it be made into an auxiliary device and the driver put in
> > drivers/reset/ so we can keep reset code in the reset directory.
>
> Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> starfive-jh7110 drivers are examples of this.
>
> > The auxiliary device creation function can also be in the
> > drivers/reset/ directory so that the clk driver calls some function
> > to create and register the device.
>
> I'm undecided about this, do you think mpfs_reset_controller_register()
> and jh7110_reset_controller_register() should rather live with the
> reset aux drivers in drivers/reset/ ?
Yes, and also mpfs_reset_read() and friends. We should pass the base
iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
then move all that code into drivers/reset with some header file
exported function to call. That way the clk driver hands over the data
without having to implement half the implementation.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 4/4] pinctrl: Implementation of the generic scmi-pinctrl driver
From: Dhruva Gole @ 2024-04-08 9:52 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Dan Carpenter, Andy Shevchenko,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan,
Oleksii Moisieiev, Tony Lindgren, Kevin Hilman,
Vignesh Raghavendra
In-Reply-To: <20240405-pinctrl-scmi-v8-4-5fc8e33871bf@nxp.com>
On Apr 05, 2024 at 09:59:35 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> scmi-pinctrl driver implements pinctrl driver interface and using
> SCMI protocol to redirect messages from pinctrl subsystem SDK to
> SCMI platform firmware, which does the changes in HW.
>
> Co-developed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> MAINTAINERS | 1 +
> drivers/pinctrl/Kconfig | 11 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-scmi.c | 564 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 577 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4b511a55101c..d8270ac6651a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21457,6 +21457,7 @@ F: drivers/cpufreq/sc[mp]i-cpufreq.c
> F: drivers/firmware/arm_scmi/
> F: drivers/firmware/arm_scpi.c
> F: drivers/hwmon/scmi-hwmon.c
> +F: drivers/pinctrl/pinctrl-scmi.c
> F: drivers/pmdomain/arm/
> F: drivers/powercap/arm_scmi_powercap.c
> F: drivers/regulator/scmi-regulator.c
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index d45657aa986a..4e6f65cf0e76 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -450,6 +450,17 @@ config PINCTRL_ROCKCHIP
> help
> This support pinctrl and GPIO driver for Rockchip SoCs.
>
> +config PINCTRL_SCMI
> + tristate "Pinctrl driver using SCMI protocol interface"
> + depends on ARM_SCMI_PROTOCOL || COMPILE_TEST
> + select PINMUX
> + select GENERIC_PINCONF
> + help
> + This driver provides support for pinctrl which is controlled
> + by firmware that implements the SCMI interface.
> + It uses SCMI Message Protocol to interact with the
> + firmware providing all the pinctrl controls.
> +
> config PINCTRL_SINGLE
> tristate "One-register-per-pin type device tree based pinctrl driver"
> depends on OF
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index 2152539b53d5..cc809669405a 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_PIC32) += pinctrl-pic32.o
> obj-$(CONFIG_PINCTRL_PISTACHIO) += pinctrl-pistachio.o
> obj-$(CONFIG_PINCTRL_RK805) += pinctrl-rk805.o
> obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o
> +obj-$(CONFIG_PINCTRL_SCMI) += pinctrl-scmi.o
> obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o
> obj-$(CONFIG_PINCTRL_ST) += pinctrl-st.o
> obj-$(CONFIG_PINCTRL_STMFX) += pinctrl-stmfx.o
> diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> new file mode 100644
> index 000000000000..0f55f000a679
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-scmi.c
> @@ -0,0 +1,564 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System Control and Power Interface (SCMI) Protocol based pinctrl driver
> + *
> + * Copyright (C) 2024 EPAM
> + * Copyright 2024 NXP
> + */
> +
> +#include <linux/device.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include <linux/pinctrl/machine.h>
> +#include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +
> +#include "pinctrl-utils.h"
> +#include "core.h"
> +#include "pinconf.h"
> +
> +#define DRV_NAME "scmi-pinctrl"
> +
> +/* Define num configs, if not large than 4 use stack, else use kcalloc */
> +#define SCMI_NUM_CONFIGS 4
> +
> +static const struct scmi_pinctrl_proto_ops *pinctrl_ops;
> +
> +struct scmi_pinctrl {
> + struct device *dev;
> + struct scmi_protocol_handle *ph;
> + struct pinctrl_dev *pctldev;
> + struct pinctrl_desc pctl_desc;
> + struct pinfunction *functions;
> + unsigned int nr_functions;
> + struct pinctrl_pin_desc *pins;
> + unsigned int nr_pins;
> +};
> +
> +static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
> +{
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> +
> + return pinctrl_ops->count_get(pmx->ph, GROUP_TYPE);
> +}
> +
> +static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev,
> + unsigned int selector)
> +{
> + int ret;
> + const char *name;
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> +
> + ret = pinctrl_ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
> + if (ret) {
> + dev_err(pmx->dev, "get name failed with err %d", ret);
> + return NULL;
> + }
> +
> + return name;
> +}
> +
> +static int pinctrl_scmi_get_group_pins(struct pinctrl_dev *pctldev,
> + unsigned int selector,
> + const unsigned int **pins,
> + unsigned int *num_pins)
> +{
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> +
> + return pinctrl_ops->group_pins_get(pmx->ph, selector, pins, num_pins);
> +}
> +
> +static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
> + .get_groups_count = pinctrl_scmi_get_groups_count,
> + .get_group_name = pinctrl_scmi_get_group_name,
> + .get_group_pins = pinctrl_scmi_get_group_pins,
> +#ifdef CONFIG_OF
> + .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> + .dt_free_map = pinconf_generic_dt_free_map,
> +#endif
> +};
> +
> +static int pinctrl_scmi_get_functions_count(struct pinctrl_dev *pctldev)
> +{
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> +
> + return pinctrl_ops->count_get(pmx->ph, FUNCTION_TYPE);
> +}
> +
> +static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
> + unsigned int selector)
> +{
> + int ret;
> + const char *name;
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> +
> + ret = pinctrl_ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
> + if (ret) {
> + dev_err(pmx->dev, "get name failed with err %d", ret);
> + return NULL;
> + }
> +
> + return name;
> +}
> +
> +static int pinctrl_scmi_get_function_groups(struct pinctrl_dev *pctldev,
> + unsigned int selector,
> + const char * const **p_groups,
> + unsigned int * const p_num_groups)
> +{
> + struct pinfunction *func;
> + const unsigned int *group_ids;
> + unsigned int num_groups;
> + const char **groups;
> + int ret, i;
Just a nit maybe, but I would be more comfortable making i with
num_groups as unsigned, because you're comparing them after all in the
loop. Also, I don't see a reason for i to become negative in any case.
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> +
> + if (!p_groups || !p_num_groups)
> + return -EINVAL;
> +
> + if (selector >= pmx->nr_functions)
> + return -EINVAL;
> +
> + func = &pmx->functions[selector];
> + if (func->ngroups)
> + goto done;
> +
> + ret = pinctrl_ops->function_groups_get(pmx->ph, selector, &num_groups,
> + &group_ids);
> + if (ret) {
> + dev_err(pmx->dev, "Unable to get function groups, err %d", ret);
> + return ret;
> + }
> + if (!num_groups)
> + return -EINVAL;
> +
> + groups = kcalloc(num_groups, sizeof(*groups), GFP_KERNEL);
> + if (!groups)
> + return -ENOMEM;
> +
> + for (i = 0; i < num_groups; i++) {
> + groups[i] = pinctrl_scmi_get_group_name(pctldev, group_ids[i]);
> + if (!groups[i]) {
> + ret = -EINVAL;
> + goto err_free;
> + }
> + }
> +
> + func->ngroups = num_groups;
> + func->groups = groups;
> +done:
> + *p_groups = func->groups;
> + *p_num_groups = func->ngroups;
> +
> + return 0;
> +
> +err_free:
> + kfree(groups);
> +
> + return ret;
> +}
> +
[...]
> +
> +static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
> + struct pinctrl_desc *desc)
> +{
> + struct pinctrl_pin_desc *pins;
> + unsigned int npins;
> + int ret, i;
better unsigned i?
> +
> + npins = pinctrl_ops->count_get(pmx->ph, PIN_TYPE);
> + /*
> + * npins will never be zero, the scmi pinctrl driver has bailed out
> + * if npins is zero.
> + */
> + pins = devm_kmalloc_array(pmx->dev, npins, sizeof(*pins), GFP_KERNEL);
> + if (!pins)
> + return -ENOMEM;
> +
> + for (i = 0; i < npins; i++) {
> + pins[i].number = i;
> + /*
> + * The memory for name is handled by the scmi firmware driver,
> + * no need free here
> + */
> + ret = pinctrl_ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
> + if (ret)
> + return dev_err_probe(pmx->dev, ret,
> + "Can't get name for pin %d", i);
> + }
> +
> + desc->npins = npins;
> + desc->pins = pins;
> + dev_dbg(pmx->dev, "got pins %u", npins);
> +
> + return 0;
> +}
> +
[...]
Unrelated and beyond scope of this patch series, but would've loved to
see concept of wakeup enable and wakeup event bits inside the pinctrl
SCMI spec like we have in pinctrl-single kernel driver. There are SOC's
out there that support wakeup IRQ's from their padconfig controllers
itself... But this is more of a feedback for the SCMI spec. Maybe a
future revision can take care of this.
The reason this needs to be
standard and not something vendor specific is because the kernel does
support a wake IRQ framework, and we will need to make this driver have
wake IRQ support if a device that supports pinctrl wakeup need to use
scmi to configure it.
Look at Table 6-2045. Description Of The Pad Configuration Register Bit
in [0] for further details for an example of a padconfig wakeup config
specially bits 29,30.
No major comments otherwise,
Reviewed-by: Dhruva Gole <d-gole@ti.com>
[0] https://www.ti.com/lit/pdf/SPRUIV7
--
Best regards,
Dhruva Gole <d-gole@ti.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] drm/mediatek: Add RENDER capability to DRM device
From: CK Hu (胡俊光) @ 2024-04-08 9:49 UTC (permalink / raw)
To: wmacek@chromium.org, linux-mediatek@lists.infradead.org,
linux-kernel@vger.kernel.org, chrome-platform@lists.linux.dev,
chunkuang.hu@kernel.org, daniel@ffwll.ch, p.zabel@pengutronix.de,
dri-devel@lists.freedesktop.org, airlied@gmail.com,
linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
angelogioacchino.delregno@collabora.com
In-Reply-To: <20240402075352.3147747-1-wmacek@chromium.org>
Hi, Wojciech:
On Tue, 2024-04-02 at 07:53 +0000, Wojciech Macek wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> DRM device shall create /dev/dri/renderXXX entry
> to allow ChromeOS VMs to access the OpenGL rendering
> offload.
I think this is the case Daniel mention in [1]. If so, this should not
upstream.
[1]
https://patchwork.kernel.org/project/dri-devel/patch/1452611750-16283-15-git-send-email-p.zabel@pengutronix.de/
Regards,
CK
>
> Signed-off-by: Wojciech Macek <wmacek@chromium.org>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 74832c213092..afa4d6391b08 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -585,7 +585,8 @@ static struct drm_gem_object
> *mtk_drm_gem_prime_import(struct drm_device *dev,
> }
>
> static const struct drm_driver mtk_drm_driver = {
> - .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
> + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC
> |
> + DRIVER_RENDER,
>
> .dumb_create = mtk_drm_gem_dumb_create,
>
> --
> 2.44.0.478.gd926399ef9-goog
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: dts: mediatek: mt7622: fix "emmc" pinctrl mux
From: Rafał Miłecki @ 2024-04-08 10:00 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, Matthias Brugger
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sean Wang,
Ryder Lee, devicetree, linux-arm-kernel, linux-mediatek,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
Value "emmc_rst" is a group name and should be part of the "groups"
property.
This fixes:
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dtb: pinctrl@10211000: emmc-pins-default:mux:function: ['emmc', 'emmc_rst'] is too long
from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt7622-pinctrl.yaml#
arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dtb: pinctrl@10211000: emmc-pins-default:mux:function: ['emmc', 'emmc_rst'] is too long
from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt7622-pinctrl.yaml#
Fixes: 3725ba3f5574 ("arm64: dts: mt7622: add pinctrl related device nodes")
Fixes: 0b6286dd96c0 ("arm64: dts: mt7622: add bananapi BPI-R64 board")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index 224bb289660c..2791de5b28f6 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -329,8 +329,8 @@ asm_sel {
/* eMMC is shared pin with parallel NAND */
emmc_pins_default: emmc-pins-default {
mux {
- function = "emmc", "emmc_rst";
- groups = "emmc";
+ function = "emmc";
+ groups = "emmc", "emmc_rst";
};
/* "NDL0","NDL1","NDL2","NDL3","NDL4","NDL5","NDL6","NDL7",
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
index 41629769bdc8..8c3e2e2578bc 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -268,8 +268,8 @@ &pio {
/* eMMC is shared pin with parallel NAND */
emmc_pins_default: emmc-pins-default {
mux {
- function = "emmc", "emmc_rst";
- groups = "emmc";
+ function = "emmc";
+ groups = "emmc", "emmc_rst";
};
/* "NDL0","NDL1","NDL2","NDL3","NDL4","NDL5","NDL6","NDL7",
--
2.35.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v1 3/4] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function
From: Anand Moon @ 2024-04-08 10:02 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Thinh Nguyen, Greg Kroah-Hartman, Krzysztof Kozlowski,
Alim Akhtar, linux-usb, linux-arm-kernel, linux-samsung-soc,
linux-kernel
In-Reply-To: <ed5153e1-f4de-4268-a9d0-e74b779d2587@wanadoo.fr>
Hi Christophe,
On Fri, 5 Apr 2024 at 21:42, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 05/04/2024 à 08:10, Anand Moon a écrit :
> > Hi Christophe, Krzysztof,
> >
> > On Mon, 4 Mar 2024 at 17:16, Anand Moon <linux.amoon@gmail.com> wrote:
> >>
> >> Hi Christophe,
> >>
> >> On Sun, 3 Mar 2024 at 00:07, Christophe JAILLET
> >> <christophe.jaillet@wanadoo.fr> wrote:
> >>>
> >>> Le 02/03/2024 à 17:48, Anand Moon a écrit :
> >>>> Hi Christophe,
> >>>>
> >>>> On Sat, 2 Mar 2024 at 21:20, Christophe JAILLET
> >>>> <christophe.jaillet@wanadoo.fr> wrote:
> >>>>>
> >>>>> Le 01/03/2024 à 20:38, Anand Moon a écrit :
> >>>>>> Use devm_regulator_bulk_get_enable() instead of open coded
> >>>>>> 'devm_regulator_get(), regulator_enable(), regulator_disable().
> >>>>>>
> >>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >>>>>> ---
> >>>>>> drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
> >>>>>> 1 file changed, 4 insertions(+), 45 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >>>>>> index 5d365ca51771..7c77f3c69825 100644
> >>>>>> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >>>>>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >>>>>> @@ -32,9 +32,6 @@ struct dwc3_exynos {
> >>>>>> struct clk *clks[DWC3_EXYNOS_MAX_CLOCKS];
> >>>>>> int num_clks;
> >>>>>> int suspend_clk_idx;
> >>>>>> -
> >>>>>> - struct regulator *vdd33;
> >>>>>> - struct regulator *vdd10;
> >>>>>> };
> >>>>>>
> >>>>>> static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> @@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> struct device_node *node = dev->of_node;
> >>>>>> const struct dwc3_exynos_driverdata *driver_data;
> >>>>>> int i, ret;
> >>>>>> + static const char * const regulators[] = { "vdd33", "vdd10" };
> >>>>>>
> >>>>>> exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
> >>>>>> if (!exynos)
> >>>>>> @@ -78,27 +76,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> if (exynos->suspend_clk_idx >= 0)
> >>>>>> clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
> >>>>>>
> >>>>>> - exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> >>>>>> - if (IS_ERR(exynos->vdd33)) {
> >>>>>> - ret = PTR_ERR(exynos->vdd33);
> >>>>>> - goto vdd33_err;
> >>>>>> - }
> >>>>>> - ret = regulator_enable(exynos->vdd33);
> >>>>>> - if (ret) {
> >>>>>> - dev_err(dev, "Failed to enable VDD33 supply\n");
> >>>>>> - goto vdd33_err;
> >>>>>> - }
> >>>>>> -
> >>>>>> - exynos->vdd10 = devm_regulator_get(dev, "vdd10");
> >>>>>> - if (IS_ERR(exynos->vdd10)) {
> >>>>>> - ret = PTR_ERR(exynos->vdd10);
> >>>>>> - goto vdd10_err;
> >>>>>> - }
> >>>>>> - ret = regulator_enable(exynos->vdd10);
> >>>>>> - if (ret) {
> >>>>>> - dev_err(dev, "Failed to enable VDD10 supply\n");
> >>>>>> - goto vdd10_err;
> >>>>>> - }
> >>>>>> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
> >>>>>> + if (ret)
> >>>>>> + return dev_err_probe(dev, ret, "Failed to enable regulators\n");
> >>>>>>
> >>>>>> if (node) {
> >>>>>> ret = of_platform_populate(node, NULL, NULL, dev);
> >>>>>> @@ -115,10 +95,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> return 0;
> >>>>>>
> >>>>>> populate_err:
> >>>>>> - regulator_disable(exynos->vdd10);
> >>>>>> -vdd10_err:
> >>>>>> - regulator_disable(exynos->vdd33);
> >>>>>> -vdd33_err:
> >>>>>> for (i = exynos->num_clks - 1; i >= 0; i--)
> >>>>>> clk_disable_unprepare(exynos->clks[i]);
> >>>>>>
> >>>>>> @@ -140,9 +116,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
> >>>>>>
> >>>>>> if (exynos->suspend_clk_idx >= 0)
> >>>>>> clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
> >>>>>> -
> >>>>>> - regulator_disable(exynos->vdd33);
> >>>>>> - regulator_disable(exynos->vdd10);
> >>>>>> }
> >>>>>>
> >>>>>> static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
> >>>>>> @@ -196,9 +169,6 @@ static int dwc3_exynos_suspend(struct device *dev)
> >>>>>> for (i = exynos->num_clks - 1; i >= 0; i--)
> >>>>>> clk_disable_unprepare(exynos->clks[i]);
> >>>>>>
> >>>>>> - regulator_disable(exynos->vdd33);
> >>>>>> - regulator_disable(exynos->vdd10);
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> Same here, I don't think that removing regulator_[en|dis]able from the
> >>>>> suspend and resume function is correct.
> >>>>>
> >>>>> The goal is to stop some hardware when the system is suspended, in order
> >>>>> to save some power.
> >>>> Ok,
> >>>>>
> >>>>> Why did you removed it?
> >>>>
> >>>> As per the description of the function devm_regulator_bulk_get_enable
> >>>>
> >>>> * This helper function allows drivers to get several regulator
> >>>> * consumers in one operation with management, the regulators will
> >>>> * automatically be freed when the device is unbound. If any of the
> >>>> * regulators cannot be acquired then any regulators that were
> >>>> * allocated will be freed before returning to the caller.
> >>>
> >>> The code in suspend/resume is not about freeing some resources. It is
> >>> about enabling/disabling some hardware to save some power.
> >>>
> >>> Think to the probe/remove functions as the software in the kernel that
> >>> knows how to handle some hardawre, and the suspend/resume as the on/off
> >>> button to power-on and off the electrical chips.
> >>>
> >>> When the system is suspended, the software is still around. But some
> >>> hardware can be set in a low consumption mode to save some power.
> >>>
> >>> IMHO, part of the code you removed changed this behaviour and increase
> >>> the power consumption when the system is suspended.
> >>>
> >>
> >> You are correct, I have changed the regulator API from
> >> devm_regulator_get_enable to devm_regulator_bulk_get_enable
> >> which changes this behavior.
> >> I will fix it in the next version.
> >>
> >>> CJ
> >
> > I could not find any example in the kernel to support
> > devm_regulator_bulk_disable
> > but here is my modified file.
> >
> > If you have any suggestions for this plz let me know.
>
> I don't think that your approach is correct, and I don't think that the
> proposed patch does what you expect it to do.
>
> Calling a devm_ function in suspend/resume functions looks really
> strange to me and is likely broken.
>
> Especially here, devm_regulator_bulk_get_enable() in the resume function
> allocates some memory that is not freed in
> devm_regulator_bulk_disable(), because the API is not designed to work
> like that. So this could generate a kind of memory leak.
>
>
> *I think that the code is good enough as-is*, but if you really want to
> change something, maybe:
> - devm_regulator_get()+regulator_enable() in the probe could be
> changed to devm_regulator_get_enable()
> - the resume/suspend function should be left as-is with
> regulator_disable()/regulator_ensable()
> - remove regulator_disable() from the error handling path of the
> probe and from the remove function.
>
> I *think* it would work.
>
No devm_regulator_get_enable use the same logic as
devm_regulator_bulk_get_enable
to enable the regulator.
[0] https://elixir.bootlin.com/linux/latest/source/drivers/regulator/devres.c#L126
So as of now I am dropping the changes on the regulator in this patch series.
> CJ
>
Thanks for your inputs.
Thanks
-Anand
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/6] usb: ehci-exynos: Use devm_clk_get_enabled() helpers
From: Anand Moon @ 2024-04-08 10:03 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Greg Kroah-Hartman, Alan Stern, Alim Akhtar, Christophe JAILLET,
Johan Hovold, linux-usb, linux-arm-kernel, linux-samsung-soc,
linux-kernel
In-Reply-To: <f52f0634-a12c-46bd-9701-e5caf4326432@linaro.org>
Hi Krzysztof, Greg.
On Thu, 4 Apr 2024 at 19:24, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/04/2024 15:52, Anand Moon wrote:
> > Hi Greg,
> >
> > On Thu, 4 Apr 2024 at 18:30, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Thu, Apr 04, 2024 at 12:43:17PM +0530, Anand Moon wrote:
> >>> The devm_clk_get_enabled() helpers:
> >>> - call devm_clk_get()
> >>> - call clk_prepare_enable() and register what is needed in order to
> >>> call clk_disable_unprepare() when needed, as a managed resource.
> >>>
> >>> This simplifies the code and avoids the calls to clk_disable_unprepare().
> >>>
> >>> While at it, use dev_err_probe consistently, and use its return value
> >>> to return the error code.
> >>>
> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >>> ---
> >>> V2: drop the clk_disable_unprepare in suspend/resume functions
> >>> fix the usb_put_hcd return before the devm_clk_get_enabled
> >>> ---
> >>> drivers/usb/host/ehci-exynos.c | 19 +++++--------------
> >>> 1 file changed, 5 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> >>> index f644b131cc0b..f00bfd0b13dc 100644
> >>> --- a/drivers/usb/host/ehci-exynos.c
> >>> +++ b/drivers/usb/host/ehci-exynos.c
> >>> @@ -159,20 +159,15 @@ static int exynos_ehci_probe(struct platform_device *pdev)
> >>>
> >>> err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
> >>> if (err)
> >>> - goto fail_clk;
> >>> -
> >>> - exynos_ehci->clk = devm_clk_get(&pdev->dev, "usbhost");
> >>> + goto fail_io;
> >>>
> >>> + exynos_ehci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost");
> >>> if (IS_ERR(exynos_ehci->clk)) {
> >>> - dev_err(&pdev->dev, "Failed to get usbhost clock\n");
> >>> - err = PTR_ERR(exynos_ehci->clk);
> >>> - goto fail_clk;
> >>> + usb_put_hcd(hcd);
> >>> + return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ehci->clk),
> >>> + "Failed to get usbhost clock\n");
> >>
> >> Why is this logic changed?
> >>
> >> If you want to call dev_err_probe(), that's great, but do NOT mix it up
> >> with a commit that does something totally different.
> >>
> >> When you say something like "while at it" in a changelog text, that is a
> >> HUGE hint that it needs to be a separate commit. Because of that reason
> >> alone, I can't take these, you know better :(
> >>
> >> thanks,
> >>
> >
> > Ok, I will improve the commit message relevant to the code changes.
>
> Please read Greg's message one more time. He did not propose to fix
> commit msg, right?
>
Ok I will drop dev_err_probe and keep the error flow as it is.
It will not break the feature..
Thanks
-Anand
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/2] mailbox: arm_mhuv3: Add driver
From: Cristian Marussi @ 2024-04-08 10:08 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-kernel, linux-arm-kernel, devicetree, sudeep.holla,
jassisinghbrar, robh+dt, krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <20240405113200.00002da9@Huawei.com>
On Fri, Apr 05, 2024 at 11:32:00AM +0100, Jonathan Cameron wrote:
> On Thu, 4 Apr 2024 07:23:47 +0100
> Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> > Add support for ARM MHUv3 mailbox controller.
> >
> > Support is limited to the MHUv3 Doorbell extension using only the PBX/MBX
> > combined interrupts.
> >
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> Drive by review (I was curious what this was :)
>
You're welcome...thanks for having a look !
>
> > diff --git a/drivers/mailbox/arm_mhuv3.c b/drivers/mailbox/arm_mhuv3.c
> > new file mode 100644
> > index 000000000000..e4125568bec0
> > --- /dev/null
> > +++ b/drivers/mailbox/arm_mhuv3.c
> > @@ -0,0 +1,1063 @@
>
> > +struct dummy_page {
> > + u8 pad[0x1000];
> > +} __packed;
> > +
> > +struct mhu3_pbx_frame_reg {
> > + struct ctrl_page ctrl;
> > + struct pdbcw_page dbcw[MHUV3_DBCW_MAX];
> > + struct dummy_page ffcw;
> > + struct dummy_page fcw;
> > + u8 pad[0xF000 - 0x4000];
> > + struct dummy_page impdef;
> > +} __packed;
> > +
> > +struct mhu3_mbx_frame_reg {
> > + struct ctrl_page ctrl;
> > + struct mdbcw_page dbcw[MHUV3_DBCW_MAX];
> > + struct dummy_page ffcw;
> > + struct dummy_page fcw;
> > + u8 pad[0xF000 - 0x4000];
> Magic, numbers, Maybe give them a definition or base them on something
> meaningful such as structure offsets?
>
Yes, it is indeed cryptic, these are the holes in the MMIO regs and
are derived from the spec...I'll see how I can rework to make this more
meaningful and better commented.
> > + struct dummy_page impdef;
> > +} __packed;
> > +
> > +/* Macro for reading a bitfield within a physically mapped packed struct */
> > +#define readl_relaxed_bitfield(_regptr, _field) \
> > + ({ \
> > + u32 _rval; \
> > + typeof(_regptr) _rptr = _regptr; \
> > + _rval = readl_relaxed(_rptr); \
> > + ((typeof(*_rptr) __force *)(&_rval))->_field; \
> > + })
> > +
> > +/* Macro for writing a bitfield within a physically mapped packed struct */
> > +#define writel_relaxed_bitfield(_value, _regptr, _field) \
> > + ({ \
> > + u32 _rval; \
> > + typeof(_regptr) _rptr = _regptr; \
> > + _rval = readl_relaxed(_rptr); \
> > + ((typeof(*_rptr) __force *)(&_rval))->_field = _value; \
> > + writel_relaxed(_rval, _rptr); \
> > + })
> Similar, yet slightly different from ones in arm_mhuv2.c? Why the differences
> and can these be shared code in a suitable header?
Yes, all the struct/bitfield based MMIO stuff is borrowed from mhuv2
since it seemed more slick than a zillions defines and bitmasks (but maybe
not exempt from issues... given what Jassi jas commented later on...), BUT
while using those I saw the opportunity to drop a parameter since v2 has a
_type arg that it can indeed derived from _regptr, so making the macros less
cumbersome to invoke....THEN sparse quicly reminded me that by using typeof()
to derive the type of the local work-variable I was also grabbing all the
related attributes attached to _regptr...namely __iomem and noderef that
triggered a bunch of warnings (unjustified since operating on a local
var NOT a real MMIO): that is the reason for the dance with __force
here, and why is not needed in v2.
> > +
> > +/* ====== MHUv3 data structures ====== */
> > +
> > +enum mhuv3_frame {
> > + PBX_FRAME,
> > + MBX_FRAME
> Trailing commas for last entries in enums unless they are in some sense terminators.
> > +};
> > +
> > +static char *mhuv3_str[] = {
> > + "PBX",
> > + "MBX"
> > +};
> > +
> > +enum mhuv3_extension_type {
> > + FIRST_EXT = 0,
> As mentioned inline, 0 is kind of default assumption for first so I wouldn't define it.
>
Indeed.
> > + DBE_EXT = FIRST_EXT,
> > + FCE_EXT,
> > + FE_EXT,
> > + MAX_EXT
> That's one past normal meeting of MAX, maybe call it COUNT, or NUM?
>
Ok.
> > +};
>
> > +static int mhuv3_doorbell_send_data(struct mhuv3 *mhu, struct mbox_chan *chan,
> > + void *arg)
> > +{
> > + int ret = 0;
> > + struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> > + struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&e->pending_lock, flags);
>
> guard() then you can do earlier returns and end up with cleaner code.
>
Yes, I'll have a look into cleanup.h at large.
>
> > + /* Only one in-flight Transfer is allowed per-doorbell */
> > + if (!(e->pending_db[priv->ch_idx] & BIT(priv->doorbell))) {
> > + e->pending_db[priv->ch_idx] |= BIT(priv->doorbell);
> > + writel_relaxed(BIT(priv->doorbell),
> > + &mhu->pbx->dbcw[priv->ch_idx].set);
> > + } else {
> > + ret = -EBUSY;
> > + }
> > + spin_unlock_irqrestore(&e->pending_lock, flags);
> > +
> > + return ret;
> > +}
> >
> > +
> > +static struct mbox_chan *mhuv3_dbe_chan_from_comb_irq_get(struct mhuv3 *mhu)
> > +{
> > + int i;
> > + struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> > + struct device *dev = mhu->mbox.dev;
> > +
> > + for (i = 0; i < MHUV3_DBCH_CMB_INT_ST_REG_CNT; i++) {
> > + unsigned int channel, db = MHUV3_INVALID_DOORBELL;
> > + u32 cmb_st, st;
> > +
> > + cmb_st = readl_relaxed(&mhu->ctrl->dbch_int_st[i]);
> > + if (!cmb_st)
> > + continue;
> > +
> > + channel = i * MHUV3_STAT_BITS + __builtin_ctz(cmb_st);
> > + if (channel >= e->max_chans) {
> > + dev_err(dev, "Invalid %s channel:%d\n",
> > + mhuv3_str[mhu->frame], channel);
>
> return here rather than breaking out the loop. It is easier to follow
> given nothing is done after the loop
>
Ok.
> > + break;
> > + }
> > +
> > + if (mhu->frame == PBX_FRAME) {
> > + unsigned long flags;
> > + u32 active_dbs, fired_dbs;
> > +
> > + st = readl_relaxed_bitfield(&mhu->pbx->dbcw[channel].int_st,
> > + tfr_ack);
> > + if (!st) {
> > + dev_warn(dev, "Spurios IRQ on %s channel:%d\n",
> Spell check. Spurious.
>
Yes.
> > + mhuv3_str[mhu->frame], channel);
> > + continue;
> > + }
> > +
> > + active_dbs = readl_relaxed(&mhu->pbx->dbcw[channel].st);
> > + spin_lock_irqsave(&e->pending_lock, flags);
> > + fired_dbs = e->pending_db[channel] & ~active_dbs;
> > + if (fired_dbs) {
> > + db = __builtin_ctz(fired_dbs);
> > + e->pending_db[channel] &= ~BIT(db);
> > + fired_dbs &= ~BIT(db);
> > + }
> > + spin_unlock_irqrestore(&e->pending_lock, flags);
> > +
> > + /* Clear TFR Ack if no more doorbells pending */
> > + if (!fired_dbs)
> > + writel_relaxed_bitfield(0x1,
> > + &mhu->pbx->dbcw[channel].int_clr,
> > + tfr_ack);
> > + } else {
> > + st = readl_relaxed(&mhu->mbx->dbcw[channel].st_msk);
> > + if (!st) {
> > + dev_warn(dev, "Spurios IRQ on %s channel:%d\n",
> > + mhuv3_str[mhu->frame], channel);
> > + continue;
> > + }
> > + db = __builtin_ctz(st);
> > + }
> > +
> > + if (db != MHUV3_INVALID_DOORBELL) {
> > + dev_dbg(dev, "Found %s ch[%d]/db[%d]\n",
> > + mhuv3_str[mhu->frame], channel, db);
> > +
> > + return &mhu->mbox.chans[channel * MHUV3_STAT_BITS + db];
> > + }
> > + }
> > +
> > + return ERR_PTR(-EIO);
> > +}
> > +
> > +static int mhuv3_dbe_init(struct mhuv3 *mhu)
> > +{
> > + struct mhuv3_extension *e;
> > + struct device *dev = mhu->mbox.dev;
> > +
> > + if (!readl_relaxed_bitfield(&mhu->ctrl->feat_spt0, dbe_spt))
> > + return 0;
> > +
> > + dev_dbg(dev, "%s: Initializing DBE Extension.\n", mhuv3_str[mhu->frame]);
> > +
> > + e = devm_kzalloc(dev, sizeof(*e), GFP_KERNEL);
> > + if (!e)
> > + return -ENOMEM;
> > +
> > + e->type = DBE_EXT;
> > + /* Note that, by the spec, the number of channels is (num_dbch + 1) */
> > + e->max_chans =
> > + readl_relaxed_bitfield(&mhu->ctrl->dbch_cfg0, num_dbch) + 1;
> > + e->mbox_of_xlate = mhuv3_dbe_mbox_of_xlate;
> > + e->combined_irq_setup = mhuv3_dbe_combined_irq_setup;
> > + e->channels_init = mhuv3_dbe_channels_init;
> > + e->chan_from_comb_irq_get = mhuv3_dbe_chan_from_comb_irq_get;
> > +
> > + mhu->tot_chans += e->max_chans * MHUV3_STAT_BITS;
> > + mhu->ext[DBE_EXT] = e;
> > +
> > + dev_info(dev, "%s: found %d DBE channels.\n",
> > + mhuv3_str[mhu->frame], e->max_chans);
> dev_dbg() probably more appropriate.
>
Ok.
> > +
> > + return 0;
> > +}
> > +
> > +static int mhuv3_fce_init(struct mhuv3 *mhu)
> > +{
> > + struct device *dev = mhu->mbox.dev;
> > +
> > + if (!readl_relaxed_bitfield(&mhu->ctrl->feat_spt0, fce_spt))
> > + return 0;
> > +
> > + dev_dbg(dev, "%s: FCE Extension not supported by driver.\n",
> > + mhuv3_str[mhu->frame]);
> > +
> > + return 0;
> > +}
> > +
> > +static int mhuv3_fe_init(struct mhuv3 *mhu)
> > +{
> > + struct device *dev = mhu->mbox.dev;
> > +
> > + if (!readl_relaxed_bitfield(&mhu->ctrl->feat_spt0, fe_spt))
> > + return 0;
> > +
> > + dev_dbg(dev, "%s: FE Extension not supported by driver.\n",
> > + mhuv3_str[mhu->frame]);
> > +
> > + return 0;
> > +}
> > +
> > +static mhuv3_extension_initializer mhuv3_extension_init[MAX_EXT] = {
> > + mhuv3_dbe_init,
> > + mhuv3_fce_init,
> > + mhuv3_fe_init,
> > +};
> > +
> > +static int mhuv3_initialize_channels(struct device *dev, struct mhuv3 *mhu)
> > +{
> > + int i, ret = 0;
> > + struct mbox_controller *mbox = &mhu->mbox;
> > +
> > + mbox->chans = devm_kcalloc(dev, mhu->tot_chans,
> > + sizeof(*mbox->chans), GFP_KERNEL);
> > + if (!mbox->chans)
> > + return -ENOMEM;
> > +
> > + for (i = FIRST_EXT; i < MAX_EXT && !ret; i++)
> Why this dance with FIRST_EXT if it is always 0? Cleaner to just use 0.
>
Ok, I'll do...I was thinking was more clear to specify what was the
start instead of a plain 0....but indeed is apparent from the context.
> > + if (mhu->ext[i])
> > + ret = mhu->ext[i]->channels_init(mhu);
> > +
> > + return ret;
> > +}
> > +
> > +static struct mbox_chan *mhuv3_mbox_of_xlate(struct mbox_controller *mbox,
> > + const struct of_phandle_args *pa)
> > +{
> > + unsigned int type, channel, param;
> > + struct mhuv3 *mhu = mhu_from_mbox(mbox);
> > +
> > + if (pa->args_count != MHUV3_MBOX_CELLS)
> > + return ERR_PTR(-EINVAL);
> > +
> > + type = pa->args[MHUV3_MBOX_CELL_TYPE];
> > + if (type >= MAX_EXT)
> > + return ERR_PTR(-EINVAL);
> > +
> > + channel = pa->args[MHUV3_MBOX_CELL_CHWN];
> > + param = pa->args[MHUV3_MBOX_CELL_PARAM];
> > +
> > + return mhu->ext[type]->mbox_of_xlate(mhu, channel, param);
> > +}
> > +
> > +static int mhuv3_frame_init(struct mhuv3 *mhu, void __iomem *regs)
> > +{
> > + int i, ret = 0;
> > + struct device *dev = mhu->mbox.dev;
> > +
> > + mhu->ctrl = regs;
> > + mhu->frame = readl_relaxed_bitfield(&mhu->ctrl->blk_id, blk_id);
> > + if (mhu->frame > MBX_FRAME) {
> > + dev_err(dev, "Invalid Frame type- %d\n", mhu->frame);
> > + return -EINVAL;
> dev_err_probe() etc (see later)
>
Yes, indeed. I've just posted a series to use dev_err_probe on the SCMI
stack and then missed completely here...my bad. I'll do.
> > + }
> > +
> > + mhu->major = readl_relaxed_bitfield(&mhu->ctrl->aidr, arch_major_rev);
> > + mhu->minor = readl_relaxed_bitfield(&mhu->ctrl->aidr, arch_minor_rev);
> > + if (mhu->major != MHUV3_MAJOR_VERSION) {
> > + dev_warn(dev, "Unsupported MHU %s block - major:%d minor:%d\n",
> > + mhuv3_str[mhu->frame], mhu->major, mhu->minor);
>
> You are treating it as an error, so why only a warning?
>
Right.
> > + return -EINVAL;
> > + }
> > + mhu->auto_op_full = !!readl_relaxed_bitfield(&mhu->ctrl->feat_spt1,
> > + auto_op_spt);
> > + /* Request the PBX/MBX to remain operational */
> > + if (mhu->auto_op_full)
> > + writel_relaxed_bitfield(0x1, &mhu->ctrl->ctrl, op_req);
> > +
> > + dev_dbg(dev, "Found MHU %s block - major:%d minor:%d\n",
> > + mhuv3_str[mhu->frame], mhu->major, mhu->minor);
> > +
> > + if (mhu->frame == PBX_FRAME)
> > + mhu->pbx = regs;
> > + else
> > + mhu->mbx = regs;
> > +
> > + for (i = FIRST_EXT; i < MAX_EXT && !ret; i++)
> > + ret = mhuv3_extension_init[i](mhu);
>
> Only dbe_init() returns any errors, so if I ready this correctly you always
> eat that error.
Yes, I'll fix the logic.
>
> > +
> > + return ret;
> > +}
> > +
> > +static irqreturn_t mhuv3_pbx_comb_interrupt(int irq, void *arg)
> > +{
> > + int ret = IRQ_NONE;
> > + unsigned int i, found = 0;
> > + struct mhuv3 *mhu = arg;
> > + struct device *dev = mhu->mbox.dev;
> > + struct mbox_chan *chan;
> > +
> > + for (i = FIRST_EXT; i < MAX_EXT; i++) {
> > + /* FCE does not participate to the PBX combined */
> > + if (i == FCE_EXT || !mhu->ext[i])
> > + continue;
> > +
> > + chan = mhu->ext[i]->chan_from_comb_irq_get(mhu);
> > + if (!IS_ERR(chan)) {
>
> if (IS_ERR(chan))
> continue;
>
> will reduce indent and give more readable code.
>
Ok.
> > + struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> > +
> > + found++;
> > + if (chan->cl) {
> > + mbox_chan_txdone(chan, 0);
> > + ret = IRQ_HANDLED;
> > + } else {
> > + dev_warn(dev,
> > + "TX Ack on UNBOUND channel (%u)\n",
> > + priv->ch_idx);
> > + }
> > + }
> > + }
> > +
> > + if (!found)
> > + dev_warn_once(dev, "Failed to find channel for the TX interrupt\n");
> > +
> > + return ret;
> > +}
> > +
> > +static irqreturn_t mhuv3_mbx_comb_interrupt(int irq, void *arg)
> > +{
> > + int ret = IRQ_NONE;
> > + unsigned int i, found = 0;
> > + struct mhuv3 *mhu = arg;
> > + struct device *dev = mhu->mbox.dev;
> > + struct mbox_chan *chan;
> > +
> > + for (i = FIRST_EXT; i < MAX_EXT; i++) {
> > + if (!mhu->ext[i])
> > + continue;
> > +
> > + /* Process any extension which could be source of the IRQ */
> > + chan = mhu->ext[i]->chan_from_comb_irq_get(mhu);
> > + if (!IS_ERR(chan)) {
>
> if (IS_ERR(chan))
> continue;
>
> is going to be easier to read.
>
ok.
> > + void *data = NULL;
> > + struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> > +
> > + found++;
> > + /* Read and acknowledge optional in-band LE data first. */
> > + if (priv->ops->read_data)
> > + data = priv->ops->read_data(mhu, chan);
> > +
> > + if (chan->cl && !IS_ERR(data)) {
> > + mbox_chan_received_data(chan, data);
> > + ret = IRQ_HANDLED;
> > + } else if (!chan->cl) {
> > + dev_warn(dev,
> > + "RX Data on UNBOUND channel (%u)\n",
> > + priv->ch_idx);
> > + } else {
> > + dev_err(dev, "Failed to read data: %lu\n",
> > + PTR_ERR(data));
> > + }
>
> I'd be tempted to factor out this code block into another function as I think
> that will allow you to deal with the errors more directly.
>
I will give a go at reworking.
> > +
> > + if (!IS_ERR(data))
> > + kfree(data);
> > +
> > + /*
> > + * Acknowledge transfer after any possible optional
> > + * out-of-band data has also been retrieved via
> > + * mbox_chan_received_data().
> > + */
> > + if (priv->ops->rx_complete)
> > + priv->ops->rx_complete(mhu, chan);
> > + }
> > + }
> > +
> > + if (!found)
> > + dev_warn_once(dev, "Failed to find channel for the RX interrupt\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int mhuv3_setup_pbx(struct mhuv3 *mhu)
> > +{
> > + struct device *dev = mhu->mbox.dev;
> > +
> > + mhu->mbox.ops = &mhuv3_sender_ops;
> > +
> > + if (mhu->cmb_irq > 0) {
> > + int ret;
> > +
> > + ret = devm_request_threaded_irq(dev, mhu->cmb_irq, NULL,
> > + mhuv3_pbx_comb_interrupt,
> > + IRQF_ONESHOT, "mhuv3-pbx", mhu);
> > + if (!ret) {
> > + int i;
> > +
> > + mhu->mbox.txdone_irq = true;
> > + mhu->mbox.txdone_poll = false;
> > +
> > + for (i = FIRST_EXT; i < MAX_EXT; i++)
> > + if (mhu->ext[i])
> > + mhu->ext[i]->combined_irq_setup(mhu);
> > +
> > + dev_dbg(dev, "MHUv3 PBX IRQs initialized.\n");
> > +
> > + return 0;
> > + }
> > +
> > + dev_err(dev, "Failed to request PBX IRQ - ret:%d", ret);
>
> If an irq was provided and it failed, I'd just return an error, not muddle on.
> Broken system. If it's not an 'error' then don't use dev_err()
>
> Papering over this leads to an odd code flow with if (!ret) so it would
> be nice not to bother unless there is a strong reason to carry on.
Well, the only reason is that when the Tx-Ack interrupt fails somehow to
be setup (or is not provided even though the spec mandates it), the
mailbox can work anyway fine, maybe on degraded performance...so here I was
trying to be kind and carry-on best-effort with a few warnings...since
I already bumped into a system where the Tx-Ack was supposedly present BUT
the wire was NOT ... but indeed better to be noisy and bailout so to have
the thing fixed early on when it happens...I'll revisit
>
>
> > + }
> > +
> > + dev_info(dev, "Using PBX in Tx polling mode.\n");
>
> That's noisy. dev_dbg() perhaps?
>
Ok, I was trying to be noisy indeed since operating without Tx-Ack can
be limiting in some circumstances and is a sort of anomaly given the
spec would expect the PBX combined interrupt to be provided (but the
mailboxes can work...)
> > + mhu->mbox.txdone_irq = false;
> > + mhu->mbox.txdone_poll = true;
> > + mhu->mbox.txpoll_period = 1;
> > +
> > + return 0;
> > +}
> > +
> > +static int mhuv3_setup_mbx(struct mhuv3 *mhu)
> > +{
> > + int ret, i;
> > + struct device *dev = mhu->mbox.dev;
> > +
> > + mhu->mbox.ops = &mhuv3_receiver_ops;
> > +
> > + if (mhu->cmb_irq <= 0) {
> > + dev_err(dev, "Missing MBX combined IRQ !\n");
> return dev_err_probe()
> here as I think it's only called form init. Sure you might not
> need the deferred handling it provides but it still leads to
> cleaner code and no one has to think about whether deferal might
> happen or not.
>
Yes I'll switch to dev_err_probe where appropriate.
> > + return -EINVAL;
> > + }
> > +
> > + ret = devm_request_threaded_irq(dev, mhu->cmb_irq, NULL,
> > + mhuv3_mbx_comb_interrupt, IRQF_ONESHOT,
> > + "mhuv3-mbx", mhu);
> > + if (ret) {
> > + dev_err(dev, "Failed to request MBX IRQ - ret:%d\n", ret);
> > + return ret;
>
> return dev_err_probe()
Ditto.
>
> > + }
> > +
> > + for (i = FIRST_EXT; i < MAX_EXT; i++)
> > + if (mhu->ext[i])
> > + mhu->ext[i]->combined_irq_setup(mhu);
> > +
> > + dev_dbg(dev, "MHUv3 MBX IRQs initialized.\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int mhuv3_irqs_init(struct mhuv3 *mhu, struct platform_device *pdev)
> > +{
> > + int ret;
> > +
> > + dev_dbg(mhu->mbox.dev, "Initializing %s block.\n", mhuv3_str[mhu->frame]);
> > +
> > + if (mhu->frame == PBX_FRAME) {
> > + mhu->cmb_irq = platform_get_irq_byname_optional(pdev, "combined");
> > + ret = mhuv3_setup_pbx(mhu);
>
> return early is both shorter and easier to follow if people
> are looking at particular paths through the function.
Ok.
>
> > + } else {
> > + mhu->cmb_irq = platform_get_irq_byname(pdev, "combined");
> > + ret = mhuv3_setup_mbx(mhu);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int mhuv3_probe(struct platform_device *pdev)
> > +{
> > + int ret;
> > + struct mhuv3 *mhu;
> > + void __iomem *regs;
> > + struct device *dev = &pdev->dev;
> > +
> > + mhu = devm_kzalloc(dev, sizeof(*mhu), GFP_KERNEL);
> > + if (!mhu)
> > + return -ENOMEM;
> > +
> > + regs = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(regs))
> > + return PTR_ERR(regs);
> > +
> > + mhu->mbox.dev = dev;
> > + ret = mhuv3_frame_init(mhu, regs);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mhuv3_irqs_init(mhu, pdev);
> > + if (ret)
> > + return ret;
> > +
> > + mhu->mbox.of_xlate = mhuv3_mbox_of_xlate;
> > + ret = mhuv3_initialize_channels(dev, mhu);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_mbox_controller_register(dev, &mhu->mbox);
> > + if (ret)
> > + dev_err(dev, "failed to register ARM MHUv3 driver %d\n", ret);
>
> Use dev_err_probe() to get a few things for free in probe time error messages message.
> return dev_err_probe(dev, reg, "failed to register ARM HMUv3 driver\n");
Ditto.
>
> return 0;
> > +
> > + platform_set_drvdata(pdev, mhu);
>
> With all devm as suggested below, can I think drop this.
>
Ok.
> > +
> > + return ret;
> > +}
> > +
> > +static int mhuv3_remove(struct platform_device *pdev)
> > +{
> > + struct mhuv3 *mhu = platform_get_drvdata(pdev);
> > +
> > + if (mhu->auto_op_full)
> > + writel_relaxed_bitfield(0x0, &mhu->ctrl->ctrl, op_req);
> > +
>
> From a quick glance probably better to use a
> devm_add_action_or_reset() so that this is turned off at
> equivalent place in remove() path as where it is turned on in _init()
>
> Only register the callback if auto_op_full()
>
> Mixing and matching devm_ and calls in remove is a path to weird
> races and corner cases so better to go all in on devm handling.
Ok, I'll switch to devm_ fully and drop remove() all along.
Thanks again for the review.
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] MAINTAINERS: mailmap: update Richard Genoud's email address
From: Richard Genoud @ 2024-04-08 10:13 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Alexandre Belloni, linux-kernel, linux-serial, Richard Genoud,
linux-arm-kernel
I'm working now at bootlin, so I'll use my bootlin address for kernel
development from now on.
Update also the yaml file for atmel-serial accordingly.
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
.mailmap | 1 +
Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml | 2 +-
MAINTAINERS | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/.mailmap b/.mailmap
index 8284692f9610..71e28f4e0d4a 100644
--- a/.mailmap
+++ b/.mailmap
@@ -524,6 +524,7 @@ Rémi Denis-Courmont <rdenis@simphalempin.com>
Ricardo Ribalda <ribalda@kernel.org> <ricardo@ribalda.com>
Ricardo Ribalda <ribalda@kernel.org> Ricardo Ribalda Delgado <ribalda@kernel.org>
Ricardo Ribalda <ribalda@kernel.org> <ricardo.ribalda@gmail.com>
+Richard Genoud <richard.genoud@bootlin.com> <richard.genoud@gmail.com>
Richard Leitner <richard.leitner@linux.dev> <dev@g0hl1n.net>
Richard Leitner <richard.leitner@linux.dev> <me@g0hl1n.net>
Richard Leitner <richard.leitner@linux.dev> <richard.leitner@skidata.com>
diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
index 65cb2e5c5eee..eb2992a447d7 100644
--- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
+++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
@@ -8,7 +8,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
maintainers:
- - Richard Genoud <richard.genoud@gmail.com>
+ - Richard Genoud <richard.genoud@bootlin.com>
properties:
compatible:
diff --git a/MAINTAINERS b/MAINTAINERS
index aea47e04c3a5..0dbdc81e46c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14363,7 +14363,7 @@ F: drivers/dma/at_xdmac.c
F: include/dt-bindings/dma/at91.h
MICROCHIP AT91 SERIAL DRIVER
-M: Richard Genoud <richard.genoud@gmail.com>
+M: Richard Genoud <richard.genoud@bootlin.com>
S: Maintained
F: Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
F: drivers/tty/serial/atmel_serial.c
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] arm64: dts: imx8mp: Align both CSI2 pixel clock
From: Alexander Stein @ 2024-04-08 10:14 UTC (permalink / raw)
To: Adam Ford, Laurent Pinchart, linux-arm-kernel
Cc: linux-arm-kernel, Conor Dooley, Fabio Estevam,
Krzysztof Kozlowski, Paul Elder, Pengutronix Kernel Team,
Rob Herring, Sascha Hauer, Shawn Guo, devicetree, imx,
Marek Vasut
In-Reply-To: <8c1935d1-7f59-4742-9659-bf87ac4b736c@denx.de>
Hi everyone,
Am Samstag, 6. April 2024, 04:58:39 CEST schrieb Marek Vasut:
> On 4/5/24 11:04 PM, Adam Ford wrote:
> > On Fri, Apr 5, 2024 at 3:43 PM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> >>
> >> Hi Marek,
> >>
> >> (CC'ing Adam)
> >>
> >> Thank you for the patch.
> >>
> >> On Fri, Apr 05, 2024 at 10:22:26PM +0200, Marek Vasut wrote:
> >>> Configure both CSI2 assigned-clock-rates the same way.
> >>> There does not seem to be any reason for keeping the
> >>> two CSI2 pixel clock set to different frequencies.
> >>
> >> There's an issue when using two cameras concurrently. This has been
> >> discussed some time ago on the linux-media mailing list, see [1]. Adam
> >> knows more than I do on this topic.
> >>
> >> [1] https://lore.kernel.org/linux-media/CAHCN7x+kymRGO2kxvN2=zLiqRjfTc3hdf3VdNVkWjsW3La0bnA@mail.gmail.com/
> >>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> ---
> >>> Cc: Conor Dooley <conor+dt@kernel.org>
> >>> Cc: Fabio Estevam <festevam@gmail.com>
> >>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> Cc: Paul Elder <paul.elder@ideasonboard.com>
> >>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> >>> Cc: Rob Herring <robh@kernel.org>
> >>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> >>> Cc: Shawn Guo <shawnguo@kernel.org>
> >>> Cc: devicetree@vger.kernel.org
> >>> Cc: imx@lists.linux.dev
> >>> Cc: linux-arm-kernel@lists.infradead.org
> >>> ---
> >>> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >>> index 1bb96e96639f2..2e9ce0c3a9815 100644
> >>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >>> @@ -1703,7 +1703,7 @@ mipi_csi_1: csi@32e50000 {
> >>> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>;
> >>> assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
> >>> <&clk IMX8MP_CLK_24M>;
> >>> - assigned-clock-rates = <266000000>;
> >>> + assigned-clock-rates = <500000000>;
> >
> > I am traveling, so I don't have the technical documents in front of
> > me, but I beleive this is an over-drive speed, and 400MHz would be the
> > single clock, standard rate. I created an imx8mm-overdrive and
> > imx8mn-overdrive dtsi file to let users who operate in overdrive mode
> > to update their clocks in one place.
> >
> > I also think this goes down if the user is running two cameras instead
> > of one. I re-read the old thread, and it's coming back to me, but
> > until I can get settled into my hotel in Germany, I won't have time to
> > review. I think the original idea was to use the lowest, conservative
> > value with the idea that people can tweak their clock settings if
> > they're only running one and if they are running in over-drive mode.
>
> MX8MPCEC does indeed read 400 MHz regular, 500 MHz overdrive.
>
> Shall we align both CSI2 ports to 400 MHz ? Currently they are one 500
> MHz and the other 266 MHz .
No, that won't do. The (industrial products) datasheet says (Table 1):
* Single camera on CSI1: 400/500 MHz in normal/overdrive mode
* Single camera on CSI2: 277 MHz
* Dual camera on CSI1 & CSI2: 266 MHz
Assuming you need CSI2 more likely in a dual camera setup only, defaulting to
266MHz seems sensible to me.
Best regards,
Alexander
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 2/4] dt-bindings: firmware: arm,scmi: support pinctrl protocol
From: Dhruva Gole @ 2024-04-08 10:14 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Dan Carpenter, Andy Shevchenko,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
In-Reply-To: <20240405-pinctrl-scmi-v8-2-5fc8e33871bf@nxp.com>
On Apr 05, 2024 at 09:59:33 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add SCMI v3.2 pinctrl protocol bindings and example.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
Reviewed-by: Dhruva Gole <d-gole@ti.com>
> .../devicetree/bindings/firmware/arm,scmi.yaml | 50 ++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 4591523b51a0..e9d3f043c4ed 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -247,6 +247,37 @@ properties:
> reg:
[...]
--
Best regards,
Dhruva
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox