* Re: [PATCH 3/3] of: Use scope based of_node_put() cleanups
From: Rob Herring @ 2024-04-05 13:00 UTC (permalink / raw)
To: Saravana Kannan; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <CAGETcx8Wd5OsHWiGSASWkQQtof0D-ScwYsvq9hWizV3DFC27gA@mail.gmail.com>
On Thu, Apr 4, 2024 at 6:22 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
> >
> > Use the relatively new scope based of_node_put() cleanup to simplify
> > function exit handling. Doing so reduces the chances of forgetting an
> > of_node_put() and simplifies error paths by avoiding the need for goto
> > statements.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > drivers/of/address.c | 60 ++++++++++++++++-----------------------------------
> > drivers/of/property.c | 22 ++++++-------------
> > 2 files changed, 26 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index ae46a3605904..f7b2d535a6d1 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -491,7 +491,6 @@ static u64 __of_translate_address(struct device_node *dev,
> > const __be32 *in_addr, const char *rprop,
> > struct device_node **host)
> > {
> > - struct device_node *parent = NULL;
> > struct of_bus *bus, *pbus;
> > __be32 addr[OF_MAX_ADDR_CELLS];
> > int na, ns, pna, pns;
> > @@ -504,7 +503,7 @@ static u64 __of_translate_address(struct device_node *dev,
> >
> > *host = NULL;
> > /* Get parent & match bus type */
> > - parent = get_parent(dev);
> > + struct device_node *parent __free(device_node) = get_parent(dev);
>
> Can we leave the variable definition where it was? We generally define
> all the variables up top. So, defining the one variable in the middle
> feels weird. I at least get when we do this inside for/if blocks. But
> randomly in the middle feels weird.
There's an 'of_node_get(dev);' before this. Ordering wise, we need to
hold the ref on the child before we get its parent. I suppose I can
also convert that to use the cleanups. I'll have to add another local
ptr to do that though.
>
> Similar comments in other places. Since both kfree() and of_put() can
> both handle NULL pointers, I'd be surprised if we HAVE to combine
> these lines.
https://lore.kernel.org/all/CAHk-=wgRHiV5VSxtfXA4S6aLUmcQYEuB67u3BJPJPtuESs1JyA@mail.gmail.com/
^ permalink raw reply
* Re: [RESEND v7 14/37] clk: Compatible with narrow registers
From: Damien Le Moal @ 2024-04-05 13:07 UTC (permalink / raw)
To: Geert Uytterhoeven, Yoshinori Sato
Cc: linux-sh, Niklas Cassel, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Turquette, Stephen Boyd, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <CAMuHMdVXvPW+3-sY2XPQ2aMcTZkK9zoMnxWeZ+PRB+VRgGszdQ@mail.gmail.com>
On 4/5/24 21:56, Geert Uytterhoeven wrote:
> Hi Sato-san,
>
> On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
> <ysato@users.sourceforge.jp> wrote:
>> divider and gate only support 32-bit registers.
>> Older hardware uses narrower registers, so I want to be able to handle
>> 8-bit and 16-bit wide registers.
>>
>> Seven clk_divider flags are used, and if I add flags for 8bit access and
>> 16bit access, 8bit will not be enough, so I expanded it to u16.
>>
>> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
>
> Thanks for the update!
>
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -26,20 +26,38 @@
>> * parent - fixed parent. No clk_set_parent support
>> */
>>
>> -static inline u32 clk_div_readl(struct clk_divider *divider)
>> -{
>> - if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> - return ioread32be(divider->reg);
>> -
>> - return readl(divider->reg);
>> +static inline u32 clk_div_read(struct clk_divider *divider)
>> +{
>> + if (divider->flags & CLK_DIVIDER_REG_8BIT)
>
> When you need curly braces in one branch of an if/else statement,
> please use curly braces in all branches (everywhere).
>
>> + return readb(divider->reg);
>> + else if (divider->flags & CLK_DIVIDER_REG_16BIT) {
And no need for an else after a return...
>> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> + return ioread16be(divider->reg);
>> + else
and here.
>> + return readw(divider->reg);
>> + } else {
>> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> + return ioread32be(divider->reg);
>> + else
here too.
>> + return readl(divider->reg);
>> + }
>> }
>
>> --- a/drivers/clk/clk-gate.c
>> +++ b/drivers/clk/clk-gate.c
>
>> @@ -137,12 +155,30 @@ struct clk_hw *__clk_hw_register_gate(struct device *dev,
>> struct clk_init_data init = {};
>> int ret = -EINVAL;
>>
>> + /* validate register size option and bit_idx */
>> if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
>> if (bit_idx > 15) {
>> pr_err("gate bit exceeds LOWORD field\n");
>> return ERR_PTR(-EINVAL);
>> }
>> }
>> + if (clk_gate_flags & CLK_GATE_REG_16BIT) {
>> + if (bit_idx > 15) {
>> + pr_err("gate bit exceeds 16 bits\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> + }
>> + if (clk_gate_flags & CLK_GATE_REG_8BIT) {
>> + if (bit_idx > 7) {
>> + pr_err("gate bit exceeds 8 bits\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> + }
>> + if ((clk_gate_flags & CLK_GATE_HIWORD_MASK) &&
>
> If you use parentheses around "a & b" here...
>
>> + clk_gate_flags & (CLK_GATE_REG_8BIT | CLK_GATE_REG_16BIT)) {
>
> please add parentheses here, too.
>
>> + pr_err("HIWORD_MASK required 32-bit register\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>>
>> /* allocate the gate */
>> gate = kzalloc(sizeof(*gate), GFP_KERNEL);
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 4a537260f655..eaa6ff1d0b2e 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np);
>> * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used for
>> * the gate register. Setting this flag makes the register accesses big
>> * endian.
>> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 8bit.
>> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 16bit.
>> */
>> struct clk_gate {
>> struct clk_hw hw;
>> void __iomem *reg;
>> u8 bit_idx;
>> - u8 flags;
>> + u32 flags;
>
> (from my comments on v6)
> There is no need to increase the size of the flags field for the gate clock.
>
>
>> spinlock_t *lock;
>> };
>>
>
>> @@ -675,13 +681,17 @@ struct clk_div_table {
>> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
>> * for the divider register. Setting this flag makes the register accesses
>> * big endian.
>> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 8bit.
>> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 16bit.
>> */
>> struct clk_divider {
>> struct clk_hw hw;
>> void __iomem *reg;
>> u8 shift;
>> u8 width;
>> - u8 flags;
>> + u16 flags;
>> const struct clk_div_table *table;
>> spinlock_t *lock;
>> };
>
>> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
>> struct device_node *np, const char *name,
>> const char *parent_name, const struct clk_hw *parent_hw,
>> const struct clk_parent_data *parent_data, unsigned long flags,
>> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
>
> "u16 clk_divider_flags", to match clk_divider.flags.
>
>> const struct clk_div_table *table, spinlock_t *lock);
>> struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
>> struct device_node *np, const char *name,
>> const char *parent_name, const struct clk_hw *parent_hw,
>> const struct clk_parent_data *parent_data, unsigned long flags,
>> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
>
> Likewise.
>
>> const struct clk_div_table *table, spinlock_t *lock);
>> struct clk *clk_register_divider_table(struct device *dev, const char *name,
>> const char *parent_name, unsigned long flags,
>> void __iomem *reg, u8 shift, u8 width,
>> - u8 clk_divider_flags, const struct clk_div_table *table,
>> + u32 clk_divider_flags, const struct clk_div_table *table,
>
> Likewise.
>
>> spinlock_t *lock);
>> /**
>> * clk_register_divider - register a divider clock with the clock framework
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH v3 19/25] media: i2c: imx258: Change register settings for variants of the sensor
From: Dave Stevenson @ 2024-04-05 13:16 UTC (permalink / raw)
To: Sakari Ailus
Cc: Luigi311, linux-media, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel
In-Reply-To: <Zg_Zl0Q2kEDJoQoe@kekkonen.localdomain>
Hi Sakari
On Fri, 5 Apr 2024 at 11:59, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> Hi Luis, Dave,
>
> On Thu, Apr 04, 2024 at 04:44:05PM -0600, Luigi311 wrote:
> > On 4/3/24 10:18, Sakari Ailus wrote:
> > > Hi Luis, Dave,
> > >
> > > On Wed, Apr 03, 2024 at 09:03:48AM -0600, git@luigi311.com wrote:
> > >> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > >>
> > >> Sony have advised that there are variants of the IMX258 sensor which
> > >> require slightly different register configuration to the mainline
> > >> imx258 driver defaults.
> > >>
> > >> There is no available run-time detection for the variant, so add
> > >> configuration via the DT compatible string.
> > >>
> > >> The Vision Components imx258 module supports PDAF, so add the
> > >> register differences for that variant
> > >>
> > >> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > >> Signed-off-by: Luis Garcia <git@luigi311.com>
> > >> ---
> > >> drivers/media/i2c/imx258.c | 48 ++++++++++++++++++++++++++++++++++----
> > >> 1 file changed, 44 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> > >> index 775d957c9b87..fa48da212037 100644
> > >> --- a/drivers/media/i2c/imx258.c
> > >> +++ b/drivers/media/i2c/imx258.c
> > >> @@ -6,6 +6,7 @@
> > >> #include <linux/delay.h>
> > >> #include <linux/i2c.h>
> > >> #include <linux/module.h>
> > >> +#include <linux/of_device.h>
> > >> #include <linux/pm_runtime.h>
> > >> #include <linux/regulator/consumer.h>
> > >> #include <media/v4l2-ctrls.h>
> > >> @@ -321,8 +322,6 @@ static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
> > >>
> > >> static const struct imx258_reg mode_common_regs[] = {
> > >> { 0x3051, 0x00 },
> > >> - { 0x3052, 0x00 },
> > >> - { 0x4E21, 0x14 },
> > >> { 0x6B11, 0xCF },
> > >> { 0x7FF0, 0x08 },
> > >> { 0x7FF1, 0x0F },
> > >> @@ -345,7 +344,6 @@ static const struct imx258_reg mode_common_regs[] = {
> > >> { 0x7FA8, 0x03 },
> > >> { 0x7FA9, 0xFE },
> > >> { 0x7B24, 0x81 },
> > >> - { 0x7B25, 0x00 },
> > >> { 0x6564, 0x07 },
> > >> { 0x6B0D, 0x41 },
> > >> { 0x653D, 0x04 },
> > >> @@ -460,6 +458,33 @@ static const struct imx258_reg mode_1048_780_regs[] = {
> > >> { 0x034F, 0x0C },
> > >> };
> > >>
> > >> +struct imx258_variant_cfg {
> > >> + const struct imx258_reg *regs;
> > >> + unsigned int num_regs;
> > >> +};
> > >> +
> > >> +static const struct imx258_reg imx258_cfg_regs[] = {
> > >> + { 0x3052, 0x00 },
> > >> + { 0x4E21, 0x14 },
> > >> + { 0x7B25, 0x00 },
> > >> +};
> > >> +
> > >> +static const struct imx258_variant_cfg imx258_cfg = {
> > >> + .regs = imx258_cfg_regs,
> > >> + .num_regs = ARRAY_SIZE(imx258_cfg_regs),
> > >> +};
> > >> +
> > >> +static const struct imx258_reg imx258_pdaf_cfg_regs[] = {
> > >> + { 0x3052, 0x01 },
> > >> + { 0x4E21, 0x10 },
> > >> + { 0x7B25, 0x01 },
> > >> +};
> > >> +
> > >> +static const struct imx258_variant_cfg imx258_pdaf_cfg = {
> > >> + .regs = imx258_pdaf_cfg_regs,
> > >> + .num_regs = ARRAY_SIZE(imx258_pdaf_cfg_regs),
> > >> +};
> > >> +
> > >> static const char * const imx258_test_pattern_menu[] = {
> > >> "Disabled",
> > >> "Solid Colour",
> > >> @@ -637,6 +662,8 @@ struct imx258 {
> > >> struct v4l2_subdev sd;
> > >> struct media_pad pad;
> > >>
> > >> + const struct imx258_variant_cfg *variant_cfg;
> > >> +
> > >> struct v4l2_ctrl_handler ctrl_handler;
> > >> /* V4L2 Controls */
> > >> struct v4l2_ctrl *link_freq;
> > >> @@ -1104,6 +1131,14 @@ static int imx258_start_streaming(struct imx258 *imx258)
> > >> return ret;
> > >> }
> > >>
> > >> + ret = imx258_write_regs(imx258, imx258->variant_cfg->regs,
> > >> + imx258->variant_cfg->num_regs);
> > >> + if (ret) {
> > >> + dev_err(&client->dev, "%s failed to set variant config\n",
> > >> + __func__);
> > >> + return ret;
> > >> + }
> > >> +
> > >> ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP,
> > >> IMX258_REG_VALUE_08BIT,
> > >> imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ?
> > >> @@ -1492,6 +1527,10 @@ static int imx258_probe(struct i2c_client *client)
> > >>
> > >> imx258->csi2_flags = ep.bus.mipi_csi2.flags;
> > >>
> > >> + imx258->variant_cfg = of_device_get_match_data(&client->dev);
> > >
> > > You'll also need to keep this working for ACPI based systems. I.e. in
> > > practice remove "of_" prefix here and add the non-PDAF variant data to the
> > > relevant ACPI ID list.
> > >
> >
> > Removing of_ is easy enough and looking at all the other commits that make
> > this change in other drivers I dont see anything else being done besides
> > adding in the .data section that is down below for both imx258 and pdaf
> > versions. Is that what you are referencing or is there some other place
> > to add variant data to ACPI ID list?
>
> Speaking of which---are you absolutely certain there are two variants of
> this sensor? Many sensors that have a different pixel pattern (PDAF pixels
> or a non-Bayer pattern) can produce Bayer data when condigured so. The fact
> that you have differing register configuration for the PDAF and non-PDAF
> cases suggests this may well be the case.
I had a discussion with our contact at Sony over the configuration,
and Soho Enterprises who made the module I have also consulted with
Sony (their main person is ex Sony himself).
There is a spec version field in the OTP which reflects the pixel
pattern. It has defined options of:
- HDR pattern
- Binning pattern
- mono
- non-PDAF
- HDR HDD
Sony can't release information on how to read that information from
the sensor OTP as it is contractually locked by contracts with Intel.
Whilst information obtained via other routes means I have checked it
on my module as HDR pattern whilst the Nautilus platform has the
non-PDAF variant, I'm not going to spoil our relationship with Sony by
releasing that.
It's possible that the Nautilus sensor will work happily with the
settings required for the PDAF variant, but I have no way of testing
that, and the registers in question are undocumented. Changing them
blindly isn't going to make any friends, and I doubt existing platform
users wish to rerun all their image quality tests on the sensor to
validate the change.
Unless Intel wish to release the information on reading the OTP, we
have no way of telling the variants apart but need different register
configurations. If there is a better way of handling that situation
than compatible strings, then I'm open to suggestions.
There's a short thread on libcamera-devel from back in 2022 where I
was looking into this [1]
Dave
[1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-June/031449.html
> >
> > >> + if (!imx258->variant_cfg)
> > >> + imx258->variant_cfg = &imx258_cfg;
> > >> +
> > >> /* Initialize subdev */
> > >> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
> > >>
> > >> @@ -1579,7 +1618,8 @@ MODULE_DEVICE_TABLE(acpi, imx258_acpi_ids);
> > >> #endif
> > >>
> > >> static const struct of_device_id imx258_dt_ids[] = {
> > >> - { .compatible = "sony,imx258" },
> > >> + { .compatible = "sony,imx258", .data = &imx258_cfg },
> > >> + { .compatible = "sony,imx258-pdaf", .data = &imx258_pdaf_cfg },
> > >> { /* sentinel */ }
> > >> };
> > >> MODULE_DEVICE_TABLE(of, imx258_dt_ids);
> > >
> >
>
> --
> Regards,
>
> Sakari Ailus
^ permalink raw reply
* Re: [RESEND v7 23/37] dt-bindings: display: sm501 register definition helper
From: Geert Uytterhoeven @ 2024-04-05 13:25 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <dac98a697c8e850054f984964af62a209f241c83.1712207606.git.ysato@users.sourceforge.jp>
Hi Sato-san,
Thanks for your patch!
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Miscellaneous Timing and Miscellaneous Control registers definition.
Please do not put raw register value definitions into DT bindings.
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> --- /dev/null
> +++ b/include/dt-bindings/display/sm501.h
> +/* Miscellaneous timing */
> +#define SM501_MISC_TIMING_EX_HOLD_0 0
> +#define SM501_MISC_TIMING_EX_HOLD_16 1
> +#define SM501_MISC_TIMING_EX_HOLD_32 2
> +#define SM501_MISC_TIMING_EX_HOLD_48 3
> +#define SM501_MISC_TIMING_EX_HOLD_64 4
> +#define SM501_MISC_TIMING_EX_HOLD_80 5
> +#define SM501_MISC_TIMING_EX_HOLD_96 6
> +#define SM501_MISC_TIMING_EX_HOLD_112 7
> +#define SM501_MISC_TIMING_EX_HOLD_128 8
> +#define SM501_MISC_TIMING_EX_HOLD_144 9
> +#define SM501_MISC_TIMING_EX_HOLD_160 10
> +#define SM501_MISC_TIMING_EX_HOLD_176 11
> +#define SM501_MISC_TIMING_EX_HOLD_192 12
> +#define SM501_MISC_TIMING_EX_HOLD_208 13
> +#define SM501_MISC_TIMING_EX_HOLD_224 14
> +#define SM501_MISC_TIMING_EX_HOLD_240 15
E.g. these are used by the (not very descriptive) "ex" property:
ex:
$ref: /schemas/types.yaml#/definitions/uint32
description: Extend bus holding time.
Please instead use an enum for the actual holding time ([ 0, 16, 32,
...]) in the DT bindings, and convert from actual holding time to
register value in the driver.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 01/18] PCI: endpoint: Introduce pci_epc_function_is_valid()
From: Niklas Cassel @ 2024-04-05 13:33 UTC (permalink / raw)
To: Damien Le Moal
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240330041928.1555578-2-dlemoal@kernel.org>
On Sat, Mar 30, 2024 at 01:19:11PM +0900, Damien Le Moal wrote:
> Introduce the epc core helper function pci_epc_function_is_valid() to
> verify that an epc pointer, a physical function number and a virtual
> function number are all valid. This avoids repeating the code pattern:
>
> if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> return err;
>
> if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
> return err;
>
> in many functions of the endpoint controller core code.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
Reviewed-by: Niklas Cassel <cassel@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree
From: Andrew Lunn @ 2024-04-05 13:35 UTC (permalink / raw)
To: Sascha Hauer
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
David Jander
In-Reply-To: <Zg_NwfxLhzdCjN87@pengutronix.de>
> Do you have a pointer why setting the delays in the phy is preferred
> over setting them in the network driver? In the end this requires us
> to have the correct phy driver whereas setting them in the network
> driver would just work for any phy driver?
One reason is that nearly every other board does it in the PHY. This
is something i've been trying to standardize on for years.
Another point is that when doing it in the MAC, most MAC drivers get
it wrong. RGMII needs 2ns delays on the clock lines. That delay can be
provided by the board, making the clock lines longer. Or the MAC or
the PHY can add the delays. phy-mode in DT tells you about what the
board requires. Your board does not have extra long clock lines, so
you need rgmii-id. If the MAC decides to implement the delay, it
should modify the value passed to the PHY to be rgmii, to indicate it
has added the delays, and the PHY should not. This is what many MAC
drivers get wrong, they don't do the masking. By standardizing on the
PHY doing the delay, we avoid this, keeping the MAC driver simple, and
probably bug free in this respect.
There is admittedly some historical confusion here. The design is not
the best. If would of been much better if the design would have both
phy-mode and mac-mode.
As for using genphy, yes it might work, but there is no real
guarantee. It is always best you drive the hardware using the driver
specific to it. Consider genphy as a fallback which might be good
enough that you can ssh into the board and install the correct
module. You should not really be using it in production.
Andrew
^ permalink raw reply
* Re: [PATCH v2 04/18] PCI: endpoint: test: Use pci_epc_mem_map/unmap()
From: Niklas Cassel @ 2024-04-05 13:37 UTC (permalink / raw)
To: Damien Le Moal
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240330041928.1555578-5-dlemoal@kernel.org>
On Sat, Mar 30, 2024 at 01:19:14PM +0900, Damien Le Moal wrote:
> Modify the endpoint test driver to use the functions pci_epc_mem_map()
> and pci_epc_mem_unmap() for the read, write and copy tests. For each
> test case, the transfer (dma or mmio) are executed in a loop to ensure
> that potentially partial mappings returned by pci_epc_mem_map() are
> correctly handled.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
You probably want to rebase this series on top of:
[1] https://lore.kernel.org/linux-pci/20240314-pci-dbi-rework-v10-0-14a45c5a938e@linaro.org/
[2] https://lore.kernel.org/linux-pci/20240320113157.322695-1-cassel@kernel.org/
As these both modify pci-epf-test.c.
AFAICT both these series [1] (DBI rework v12, not v10) and [2] are fully
reviewed and seem to be ready to go.
They just seem to take a really long time to be picked up.
Kind regards,
Niklas
^ permalink raw reply
* Re: [PATCH v2 06/18] PCI: endpoint: test: Implement link_down event operation
From: Niklas Cassel @ 2024-04-05 13:39 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Damien Le Moal, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240403074823.GE25309@thinkpad>
On Wed, Apr 03, 2024 at 01:18:23PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Mar 30, 2024 at 01:19:16PM +0900, Damien Le Moal wrote:
> > Implement the link_down event operation to stop the command execution
> > delayed work when the endpoint controller notifies a link down event.
> >
> > Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>
> This patch is already part of another series I posted [1] and under review. So
> this can be dropped.
>
> - Mani
>
> [1] https://lore.kernel.org/linux-pci/20240401-pci-epf-rework-v2-9-970dbe90b99d@linaro.org/
Mani, your patch does not use _sync(),
so I don't think that we can simply drop this patch.
Kind regards,
Niklas
>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > drivers/pci/endpoint/functions/pci-epf-test.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index ab40c3182677..e6d4e1747c9f 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -824,9 +824,19 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
> > return 0;
> > }
> >
> > +static int pci_epf_test_link_down(struct pci_epf *epf)
> > +{
> > + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> > +
> > + cancel_delayed_work_sync(&epf_test->cmd_handler);
> > +
> > + return 0;
> > +}
> > +
> > static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> > .core_init = pci_epf_test_core_init,
> > .link_up = pci_epf_test_link_up,
> > + .link_down = pci_epf_test_link_down,
> > };
> >
> > static int pci_epf_test_alloc_space(struct pci_epf *epf)
> > --
> > 2.44.0
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v8 0/4] ASoc: PCM6240: mixer-test report
From: Mark Brown @ 2024-04-05 13:40 UTC (permalink / raw)
To: Shenghao Ding
Cc: linux-kernel, lgirdwood, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-sound, devicetree, perex, tiwai, 13916275206,
mohit.chawla, soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal,
aviel, hnagalla, praneeth, Baojun.Xu
In-Reply-To: <20240403003159.389-1-shenghao-ding@ti.com>
[-- Attachment #1: Type: text/plain, Size: 611 bytes --]
On Wed, Apr 03, 2024 at 08:31:54AM +0800, Shenghao Ding wrote:
> mixer-test report:
> root@am335x-evm:/bin# mixer-test
> TAP version 13
> # Card 0 - TI BeagleBone Black (TI BeagleBone Black)
> 1..7
> ok 1 get_value.0.0
> # 0.0 pcmd3180-i2c-2 Profile id
> ok 2 name.0.0
> ok 3 write_default.0.0
> ok 4 write_valid.0.0
> ok 5 write_invalid.0.0
> ok 6 event_missing.0.0
> ok 7 event_spurious.0.0
> # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
> root@am335x-evm:/bin#
None of the additional %s-i2c-%d-dev%d-ch%d-ana-gain type controls
appear to have shown up here - what's the story there?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 05/18] PCI: endpoint: test: Synchronously cancel command handler work
From: Niklas Cassel @ 2024-04-05 13:41 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Damien Le Moal, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240403074702.GD25309@thinkpad>
On Wed, Apr 03, 2024 at 01:17:02PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Mar 30, 2024 at 01:19:15PM +0900, Damien Le Moal wrote:
> > Replace the call to cancel_delayed_work() with a call to
> > cancel_delayed_work_sync() in pci_epf_test_unbind(). This ensures that
> > the command handler is really stopped when proceeding with dma and bar
> > cleanup.
> >
> > Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> - Mani
>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
Reviewed-by: Niklas Cassel <cassel@kernel.org>
^ permalink raw reply
* Re: [RESEND v7 28/37] dt-bindings: soc: renesas: sh: Add SH7751 based target
From: Geert Uytterhoeven @ 2024-04-05 13:44 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <3c2937039026fdb827709b2584528aca263f2668.1712207606.git.ysato@users.sourceforge.jp>
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Thanks for the update!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/renesas/sh.yaml
> + compatible:
> + oneOf:
As adding more SoCs is expected, having oneOf from the start is fine.
> + - description: SH7751R based platform
> + items:
> + - enum:
> + - renesas,rts7751r2d # Renesas SH4 2D graphics board
> + - iodata,landisk # LANDISK HDL-U
> + - iodata,usl-5p # USL-5P
> + - const: renesas,sh7751r
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v3 2/2] phy: add driver for MediaTek XFI T-PHY
From: Vinod Koul @ 2024-04-05 14:06 UTC (permalink / raw)
To: Daniel Golle
Cc: Bc-bocun Chen, Steven Liu, John Crispin, Chunfeng Yun,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Qingfang Deng, SkyLake Huang, Philipp Zabel, linux-arm-kernel,
linux-mediatek, linux-phy, devicetree, linux-kernel, netdev
In-Reply-To: <ZgXPSrcj0egvzmS6@makrotopia.org>
On 28-03-24, 20:12, Daniel Golle wrote:
> Hi Vinod,
>
> thank you for taking your time to review my submission!
>
> On Fri, Mar 29, 2024 at 12:22:47AM +0530, Vinod Koul wrote:
> > On 10-02-24, 02:10, Daniel Golle wrote:
> > > Add driver for MediaTek's XFI T-PHY which can be found in the MT7988
> >
> > What does XFI mean?
>
> https://en.wikipedia.org/wiki/XFP_transceiver#XFI
>
> I chose this name because names of functions dealing with the phy in
> the vendor driver are prefixed "xfi_pextp_".
> The register space used by the phy is called "pextp", which could be
> read as "_P_CI _ex_press _T_-_P_hy", and that is quite misleading as
> this phy isn't used for anything related to PCIe, so I wanted to find
> a better name.
>
> XFI is still somehow related (as in: you would find the relevant
> places using grep in the vendor driver when looking for that) and
> seemed to at least somehow be aligned with the function of that phy:
> Dealing with (up to) 10 Gbit/s Ethernet SerDes signals.
>
> MediaTek calls phys with more than one potential use T-PHY or X-PHY:
> The capital letter 'T' graphically connects 3 points, two of them
> being on the upper side representing the internal components and one
> on the lower side representing the single external interface.
>
> Other vendors (like Marvell) call such things "combo phys".
>
> Anyway, if anyone has better ideas regarding the naming, now is the
> moment to speak up ;)
This is fine. Combo phys are more common for ones dealing with multiple
components. I am fine either way. But it is good to document why this
was named as such
>
>
> >
> > > SoC. The XFI T-PHY is a 10 Gigabit/s Ethernet SerDes PHY with muxes on
> > > the internal side to be used with either USXGMII PCS or LynxI PCS,
> > > depending on the selected PHY interface mode.
> > >
> > > The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
> > > PHY_INTERFACE_MODE_* corresponding to the supported modes:
> > >
> > > * USXGMII \
> > > * 10GBase-R }- USXGMII PCS - XGDM \
> > > * 5GBase-R / \
> > > }- Ethernet MAC
> > > * 2500Base-X \ /
> > > * 1000Base-X }- LynxI PCS - GDM /
> > > * Cisco SGMII (MAC side) /
> > >
> > > In order to work-around a performance issue present on the first of
> > > two XFI T-PHYs present in MT7988, special tuning is applied which can be
> > > selected by adding the 'mediatek,usxgmii-performance-errata' property to
> > > the device tree node.
> > >
> > > There is no documentation for most registers used for the
> > > analog/tuning part, however, most of the registers have been partially
> > > reverse-engineered from MediaTek's SDK implementation (an opaque
> > > sequence of 32-bit register writes) and descriptions for all relevant
> > > digital registers and bits such as resets and muxes have been supplied
> > > by MediaTek.
> > >
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > ---
> > > v3: no changes
> > > v2:
> > > * use IO helpers from mtk-io.h instead of rolling my own
> > > * use devm_clk_bulk_get()
> > > * yse devm_platform_ioremap_resource()
> > > * unify name and description everywhere
> > > * invert bool is_xgmii into bool use_lynxi_pcs and add comments
> > > describing the meaning of each of the stack variables
> > > * not much we can do about remaining magic values unless MTK provides
> > > definitions for them
> > >
> > >
> > > MAINTAINERS | 1 +
> > > drivers/phy/mediatek/Kconfig | 12 +
> > > drivers/phy/mediatek/Makefile | 1 +
> > > drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 360 ++++++++++++++++++++++++
> > > 4 files changed, 374 insertions(+)
> > > create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 4be2fd097f261..616b86e3e62fd 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -13776,6 +13776,7 @@ L: netdev@vger.kernel.org
> > > S: Maintained
> > > F: drivers/net/phy/mediatek-ge-soc.c
> > > F: drivers/net/phy/mediatek-ge.c
> > > +F: drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > >
> > > MEDIATEK I2C CONTROLLER DRIVER
> > > M: Qii Wang <qii.wang@mediatek.com>
> > > diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> > > index 3849b7c87d287..117d0e84c7360 100644
> > > --- a/drivers/phy/mediatek/Kconfig
> > > +++ b/drivers/phy/mediatek/Kconfig
> > > @@ -13,6 +13,18 @@ config PHY_MTK_PCIE
> > > callback for PCIe GEN3 port, it supports software efuse
> > > initialization.
> > >
> > > +config PHY_MTK_XFI_TPHY
> > > + tristate "MediaTek 10GE SerDes XFI T-PHY driver"
> > > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > > + depends on OF && OF_ADDRESS
> >
> > why both, is OF not enough?
>
> As we are already also depending on HAS_IOMEM what is left there is
> basically just a !SPARC dependency.
> And that is probably a historic left-over and (according to commit
> 5ab5fc7e35705c from 2010...) should be re-evaluated. I'm happy to drop
> OF_ADDRESS and keep only HAS_IOMEM, and we shall see if any of the
> COMPILE_TESTs actually fails, given that everyone is fine with that.
Yeah HAS_IOMEM would be required for it to get compiled on diff archs. I
think OF should suffice... wdyt?
>
> >
> > > + depends on HAS_IOMEM
> > > + select GENERIC_PHY
> > > + help
> > > + Say 'Y' here to add support for MediaTek XFI T-PHY driver.
> > > + The driver provides access to the Ethernet SerDes T-PHY supporting
> > > + 1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
> > > + via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
> > > +
> > > config PHY_MTK_TPHY
> > > tristate "MediaTek T-PHY Driver"
> > > depends on ARCH_MEDIATEK || COMPILE_TEST
> > > diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> > > index f6e24a47e0815..1b8088df71e84 100644
> > > --- a/drivers/phy/mediatek/Makefile
> > > +++ b/drivers/phy/mediatek/Makefile
> > > @@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
> > > obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> > > obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> > > obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> > > +obj-$(CONFIG_PHY_MTK_XFI_TPHY) += phy-mtk-xfi-tphy.o
> > >
> > > phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> > > phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> > > diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > > new file mode 100644
> > > index 0000000000000..551d6cee33f94
> > > --- /dev/null
> > > +++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > > @@ -0,0 +1,360 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/* MediaTek 10GE SerDes XFI T-PHY driver
> > > + *
> > > + * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
> > > + * Bc-bocun Chen <bc-bocun.chen@mediatek.com>
> > > + * based on mtk_usxgmii.c and mtk_sgmii.c found in MediaTek's SDK (GPL-2.0)
> > > + * Copyright (c) 2022 MediaTek Inc.
> > > + * Author: Henry Yen <henry.yen@mediatek.com>
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/device.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/of.h>
> > > +#include <linux/io.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/phy.h>
> > > +#include <linux/phy/phy.h>
> > > +
> > > +#include "phy-mtk-io.h"
> > > +
> > > +#define MTK_XFI_TPHY_NUM_CLOCKS 2
> > > +
> > > +#define REG_DIG_GLB_70 0x0070
> > > +#define XTP_PCS_RX_EQ_IN_PROGRESS(x) FIELD_PREP(GENMASK(25, 24), (x))
> > > +#define XTP_PCS_MODE_MASK GENMASK(17, 16)
> > > +#define XTP_PCS_MODE(x) FIELD_PREP(GENMASK(17, 16), (x))
> > > +#define XTP_PCS_RST_B BIT(15)
> > > +#define XTP_FRC_PCS_RST_B BIT(14)
> > > +#define XTP_PCS_PWD_SYNC_MASK GENMASK(13, 12)
> > > +#define XTP_PCS_PWD_SYNC(x) FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
> > > +#define XTP_PCS_PWD_ASYNC_MASK GENMASK(11, 10)
> > > +#define XTP_PCS_PWD_ASYNC(x) FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
> > > +#define XTP_FRC_PCS_PWD_ASYNC BIT(8)
> > > +#define XTP_PCS_UPDT BIT(4)
> > > +#define XTP_PCS_IN_FR_RG BIT(0)
> > > +
> > > +#define REG_DIG_GLB_F4 0x00f4
> > > +#define XFI_DPHY_PCS_SEL BIT(0)
> > > +#define XFI_DPHY_PCS_SEL_SGMII FIELD_PREP(XFI_DPHY_PCS_SEL, 1)
> > > +#define XFI_DPHY_PCS_SEL_USXGMII FIELD_PREP(XFI_DPHY_PCS_SEL, 0)
> > > +#define XFI_DPHY_AD_SGDT_FRC_EN BIT(5)
> > > +
> > > +#define REG_DIG_LN_TRX_40 0x3040
> > > +#define XTP_LN_FRC_TX_DATA_EN BIT(29)
> > > +#define XTP_LN_TX_DATA_EN BIT(28)
> > > +
> > > +#define REG_DIG_LN_TRX_B0 0x30b0
> > > +#define XTP_LN_FRC_TX_MACCK_EN BIT(5)
> > > +#define XTP_LN_TX_MACCK_EN BIT(4)
> > > +
> > > +#define REG_ANA_GLB_D0 0x90d0
> > > +#define XTP_GLB_USXGMII_SEL_MASK GENMASK(3, 1)
> > > +#define XTP_GLB_USXGMII_SEL(x) FIELD_PREP(GENMASK(3, 1), (x))
> > > +#define XTP_GLB_USXGMII_EN BIT(0)
> > > +
> > > +struct mtk_xfi_tphy {
> > > + void __iomem *base;
> > > + struct device *dev;
> > > + struct reset_control *reset;
> > > + struct clk_bulk_data clocks[MTK_XFI_TPHY_NUM_CLOCKS];
> > > + bool da_war;
> > > +};
> > > +
> > > +static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
> > > + phy_interface_t interface)
> > > +{
> > > + /* Override 10GBase-R tuning value if work-around is selected */
> > > + bool da_war = (xfi_tphy->da_war && (interface == PHY_INTERFACE_MODE_10GBASER));
> >
> > why do you need braces around this?
>
> Just for readability. They can safely be removed.
>
> >
> > > + /* Bools to make setting up values for specific PHY speeds easier */
> > > + bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
> > > + bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
> > > + interface == PHY_INTERFACE_MODE_SGMII);
> > > + bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
> > > + interface == PHY_INTERFACE_MODE_USXGMII);
> > > + bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
> > > + /* Bool to configure input mux to either
> > > + * - USXGMII PCS (64b/66b coding) for 5G/10G
> > > + * - LynxI PCS (8b/10b coding) for 1G/2.5G
> > > + */
> > > + bool use_lynxi_pcs = (is_1g || is_2p5g);
> >
> > This is quite terrible to read, how about declaring variables first and
> > then doing the initialization?
>
> Ack.
>
> >
> > > +
> > > + dev_dbg(xfi_tphy->dev, "setting up for mode %s\n", phy_modes(interface));
> > > +
> > > + /* Setup PLL setting */
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x9024, 0x100000, is_10g ? 0x0 : 0x100000);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x2020, 0x202000, is_5g ? 0x202000 : 0x0);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x2030, 0x500, is_1g ? 0x0 : 0x500);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x2040, 0x340000, is_1g ? 0x200000 : 0x140000);
> >
> > magic numbers?
>
> Yes, and not much we can do about them. According to MTK engineers (in
> Cc) they also don't know what those numbers really mean in detail and
> have only been given sequences of magic register writes for each
> interface mode ([1], [2], [3], [4], [5]) by the upstream IP supplier
> of the PHY. I then compared those write sequences with each others,
> and observed the behavior of each register (as in: read their value
> before and after the write operation; all of them read back the value
> written to them) and rewrote the initialization as one function only
> changing the bits actually needed (instead of always writing the complete
> 32-bit value). I've made sure that everything still works and Bc-bocun
> Chen of MediaTek (also in Cc) then helped to label at least some of
> the registers and bits there in as far as they are understood by
> MediaTek.
>
> [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c#172
> [2]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c#284
> [3]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_usxgmii.c#132
> [4]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_usxgmii.c#246
> [5]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_usxgmii.c#360
Okay lets document the source of magic values so that people may refer
to these
>
> >
> > > +
> > > + /* Setup RXFE BW setting */
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x50f0, 0xc10, is_1g ? 0x410 : is_5g ? 0x800 : 0x400);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
> > > +
> > > + /* Setup RX CDR setting */
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x5070, 0x670000, is_5g ? 0x620000 : 0x50000);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x5074, 0x180000, is_5g ? 0x180000 : 0x0);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x5078, 0xf000400, is_5g ? 0x8000000 :
> > > + 0x7000400);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x507c, 0x5000500, is_5g ? 0x4000400 :
> > > + 0x1000100);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x5080, 0x1410, is_1g ? 0x400 : is_5g ? 0x1010 : 0x0);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x5084, 0x30300, is_1g ? 0x30300 :
> > > + is_5g ? 0x30100 :
> > > + 0x100);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x5088, 0x60200, is_1g ? 0x20200 :
> > > + is_5g ? 0x40000 :
> > > + 0x20000);
> > > +
> > > + /* Setting RXFE adaptation range setting */
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x50a8, 0xee0000, is_5g ? 0x800000 :
> > > + 0x6e0000);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x6004, 0x190000, is_5g ? 0x0 : 0x190000);
> > > +
> > > + if (is_10g)
> > > + writel(0x01423342, xfi_tphy->base + 0x00f8);
> > > + else if (is_5g)
> > > + writel(0x00a132a1, xfi_tphy->base + 0x00f8);
> > > + else if (is_2p5g)
> > > + writel(0x009c329c, xfi_tphy->base + 0x00f8);
> > > + else
> > > + writel(0x00fa32fa, xfi_tphy->base + 0x00f8);
> > > +
> > > + /* Force SGDT_OUT off and select PCS */
> > > + mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_F4,
> > > + XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
> > > + XFI_DPHY_AD_SGDT_FRC_EN |
> > > + (use_lynxi_pcs ? XFI_DPHY_PCS_SEL_SGMII :
> > > + XFI_DPHY_PCS_SEL_USXGMII));
> > > +
> > > + /* Force GLB_CKDET_OUT */
> > > + mtk_phy_set_bits(xfi_tphy->base + 0x0030, 0xc00);
> > > +
> > > + /* Force AEQ on */
> > > + writel(XTP_PCS_RX_EQ_IN_PROGRESS(2) | XTP_PCS_PWD_SYNC(2) | XTP_PCS_PWD_ASYNC(2),
> > > + xfi_tphy->base + REG_DIG_GLB_70);
> > > +
> > > + usleep_range(1, 5);
> > > +
> > > + /* Setup TX DA default value */
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x30b0, 0x30, 0x20);
> > > + writel(0x00008a01, xfi_tphy->base + 0x3028);
> > > + writel(0x0000a884, xfi_tphy->base + 0x302c);
> > > + writel(0x00083002, xfi_tphy->base + 0x3024);
> > > +
> > > + /* Setup RG default value */
> > > + if (use_lynxi_pcs) {
> > > + writel(0x00011110, xfi_tphy->base + 0x3010);
> > > + writel(0x40704000, xfi_tphy->base + 0x3048);
> > > + } else {
> > > + writel(0x00022220, xfi_tphy->base + 0x3010);
> > > + writel(0x0f020a01, xfi_tphy->base + 0x5064);
> > > + writel(0x06100600, xfi_tphy->base + 0x50b4);
> > > + if (interface == PHY_INTERFACE_MODE_USXGMII)
> > > + writel(0x40704000, xfi_tphy->base + 0x3048);
> > > + else
> > > + writel(0x47684100, xfi_tphy->base + 0x3048);
> > > + }
> > > +
> > > + if (is_1g)
> > > + writel(0x0000c000, xfi_tphy->base + 0x3064);
> > > +
> > > + /* Setup RX EQ initial value */
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x3050, 0xa8000000,
> > > + (interface != PHY_INTERFACE_MODE_10GBASER) ? 0xa8000000 : 0x0);
> > > + mtk_phy_update_bits(xfi_tphy->base + 0x3054, 0xaa,
> > > + (interface != PHY_INTERFACE_MODE_10GBASER) ? 0xaa : 0x0);
> > > +
> > > + if (!use_lynxi_pcs)
> > > + writel(0x00000f00, xfi_tphy->base + 0x306c);
> > > + else if (is_2p5g)
> > > + writel(0x22000f00, xfi_tphy->base + 0x306c);
> > > + else
> > > + writel(0x20200f00, xfi_tphy->base + 0x306c);
> > > +
> > > + mtk_phy_update_bits(xfi_tphy->base + 0xa008, 0x10000, da_war ? 0x10000 : 0x0);
> > > +
> > > + mtk_phy_update_bits(xfi_tphy->base + 0xa060, 0x50000, use_lynxi_pcs ? 0x50000 : 0x40000);
> > > +
> > > + /* Setup PHYA speed */
> > > + mtk_phy_update_bits(xfi_tphy->base + REG_ANA_GLB_D0,
> > > + XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
> > > + is_10g ? XTP_GLB_USXGMII_SEL(0) :
> > > + is_5g ? XTP_GLB_USXGMII_SEL(1) :
> > > + is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
> > > + XTP_GLB_USXGMII_SEL(3));
> > > + mtk_phy_set_bits(xfi_tphy->base + REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
> > > +
> > > + /* Release reset */
> > > + mtk_phy_set_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > + XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
> > > + usleep_range(150, 500);
> > > +
> > > + /* Switch to P0 */
> > > + mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > + XTP_PCS_IN_FR_RG |
> > > + XTP_FRC_PCS_PWD_ASYNC |
> > > + XTP_PCS_PWD_ASYNC_MASK |
> > > + XTP_PCS_PWD_SYNC_MASK |
> > > + XTP_PCS_UPDT,
> > > + XTP_PCS_IN_FR_RG |
> > > + XTP_FRC_PCS_PWD_ASYNC |
> > > + XTP_PCS_UPDT);
> > > + usleep_range(1, 5);
> > > +
> > > + mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_70, XTP_PCS_UPDT);
> > > + usleep_range(15, 50);
> > > +
> > > + if (use_lynxi_pcs) {
> > > + /* Switch to Gen2 */
> > > + mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > + XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> > > + XTP_PCS_MODE(1) | XTP_PCS_UPDT);
> > > + } else {
> > > + /* Switch to Gen3 */
> > > + mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > + XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> > > + XTP_PCS_MODE(2) | XTP_PCS_UPDT);
> > > + }
> > > + usleep_range(1, 5);
> > > +
> > > + mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_70, XTP_PCS_UPDT);
> > > +
> > > + usleep_range(100, 500);
> > > +
> > > + /* Enable MAC CK */
> > > + mtk_phy_set_bits(xfi_tphy->base + REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
> > > + mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_F4, XFI_DPHY_AD_SGDT_FRC_EN);
> > > +
> > > + /* Enable TX data */
> > > + mtk_phy_set_bits(xfi_tphy->base + REG_DIG_LN_TRX_40,
> > > + XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
> > > + usleep_range(400, 1000);
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode mode, int
> > > + submode)
> > > +{
> > > + struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > + if (mode != PHY_MODE_ETHERNET)
> > > + return -EINVAL;
> > > +
> > > + switch (submode) {
> > > + case PHY_INTERFACE_MODE_1000BASEX:
> > > + case PHY_INTERFACE_MODE_2500BASEX:
> > > + case PHY_INTERFACE_MODE_SGMII:
> > > + case PHY_INTERFACE_MODE_5GBASER:
> > > + case PHY_INTERFACE_MODE_10GBASER:
> > > + case PHY_INTERFACE_MODE_USXGMII:
> > > + mtk_xfi_tphy_setup(xfi_tphy, submode);
> > > + return 0;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_reset(struct phy *phy)
> > > +{
> > > + struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > + reset_control_assert(xfi_tphy->reset);
> > > + usleep_range(100, 500);
> > > + reset_control_deassert(xfi_tphy->reset);
> > > + usleep_range(1, 10);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_power_on(struct phy *phy)
> > > +{
> > > + struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > + return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_power_off(struct phy *phy)
> > > +{
> > > + struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > + clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct phy_ops mtk_xfi_tphy_ops = {
> > > + .power_on = mtk_xfi_tphy_power_on,
> > > + .power_off = mtk_xfi_tphy_power_off,
> > > + .set_mode = mtk_xfi_tphy_set_mode,
> > > + .reset = mtk_xfi_tphy_reset,
> > > + .owner = THIS_MODULE,
> > > +};
> > > +
> > > +static int mtk_xfi_tphy_probe(struct platform_device *pdev)
> > > +{
> > > + struct device_node *np = pdev->dev.of_node;
> > > + struct phy_provider *phy_provider;
> > > + struct mtk_xfi_tphy *xfi_tphy;
> > > + struct phy *phy;
> > > + int ret;
> > > +
> > > + if (!np)
> > > + return -ENODEV;
> > > +
> > > + xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
> > > + if (!xfi_tphy)
> > > + return -ENOMEM;
> > > +
> > > + xfi_tphy->base = devm_platform_ioremap_resource(pdev, 0);
> > > + if (IS_ERR(xfi_tphy->base))
> > > + return PTR_ERR(xfi_tphy->base);
> > > +
> > > + xfi_tphy->dev = &pdev->dev;
> > > + xfi_tphy->clocks[0].id = "topxtal";
> > > + xfi_tphy->clocks[1].id = "xfipll";
> > > + ret = devm_clk_bulk_get(&pdev->dev, MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > > + if (IS_ERR(xfi_tphy->reset))
> > > + return PTR_ERR(xfi_tphy->reset);
> > > +
> > > + xfi_tphy->da_war = of_property_read_bool(np, "mediatek,usxgmii-performance-errata");
> > > +
> > > + phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
> > > + if (IS_ERR(phy))
> > > + return PTR_ERR(phy);
> > > +
> > > + phy_set_drvdata(phy, xfi_tphy);
> > > + phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
> > > +
> > > + return PTR_ERR_OR_ZERO(phy_provider);
> > > +}
> > > +
> > > +static const struct of_device_id mtk_xfi_tphy_match[] = {
> > > + { .compatible = "mediatek,mt7988-xfi-tphy", },
> > > + { /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
> > > +
> > > +static struct platform_driver mtk_xfi_tphy_driver = {
> > > + .probe = mtk_xfi_tphy_probe,
> > > + .driver = {
> > > + .name = "mtk-xfi-tphy",
> > > + .of_match_table = mtk_xfi_tphy_match,
> > > + },
> > > +};
> > > +module_platform_driver(mtk_xfi_tphy_driver);
> > > +
> > > +MODULE_DESCRIPTION("MediaTek 10GE SerDes XFI T-PHY driver");
> > > +MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
> > > +MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.43.0
> >
> > --
> > ~Vinod
--
~Vinod
^ permalink raw reply
* [PATCH 0/4] Introduce msm8916 based Motorola devices
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin,
Ruby Iris Juric, Stephan Gerhold, Wiktor Strzębała,
Valérie Roux, Martijn Braam
This series introduces a set of msm8916 bsed Motorola devices:
- Moto G4 Play (harpia)
- Moto G 2015 (osprey)
- Moto E 2015 LTE (surnia)
The submission brings up the following features:
- eMMC and SD;
- Buttons;
- Touchscreen;
- USB;
- Fuel Gauge;
- Sound;
- Accelerometer (harpia only).
Since the devices share a lot of similarities, the common parts of the
DT are separated out into a dedicated dtsi, introduced with the first
device.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
Martijn Braam (1):
arm64: dts: qcom: Add Motorola Moto G 2015 (osprey)
Nikita Travkin (1):
dt-bindings: arm: qcom: Add msm8916 based Motorola devices
Ruby Iris Juric (1):
arm64: dts: qcom: Add device tree for Motorola Moto G4 Play (harpia)
Wiktor Strzębała (1):
arm64: dts: qcom: Add Motorola Moto E 2015 LTE (surnia)
Documentation/devicetree/bindings/arm/qcom.yaml | 3 +
arch/arm64/boot/dts/qcom/Makefile | 3 +
.../boot/dts/qcom/msm8916-motorola-common.dtsi | 161 +++++++++++++++++++++
.../boot/dts/qcom/msm8916-motorola-harpia.dts | 147 +++++++++++++++++++
.../boot/dts/qcom/msm8916-motorola-osprey.dts | 105 ++++++++++++++
.../boot/dts/qcom/msm8916-motorola-surnia.dts | 83 +++++++++++
6 files changed, 502 insertions(+)
---
base-commit: 29493ca7d6b1d3fdc224467c422ac9bdf6d7a252
change-id: 20240405-msm8916-moto-init-640862b8f57c
Best regards,
--
Nikita Travkin <nikita@trvn.ru>
^ permalink raw reply
* [PATCH 1/4] dt-bindings: arm: qcom: Add msm8916 based Motorola devices
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin
In-Reply-To: <20240405-msm8916-moto-init-v1-0-502b58176d34@trvn.ru>
Add compatible values for the msm8916 based Motorola smartphones.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
Documentation/devicetree/bindings/arm/qcom.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 1646e5bd23d8..29ff7c833909 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -197,6 +197,9 @@ properties:
- huawei,g7
- longcheer,l8910
- longcheer,l8150
+ - motorola,harpia
+ - motorola,osprey
+ - motorola,surnia
- qcom,msm8916-mtp
- samsung,a3u-eur
- samsung,a5u-eur
--
2.44.0
^ permalink raw reply related
* [PATCH 2/4] arm64: dts: qcom: Add device tree for Motorola Moto G4 Play (harpia)
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin,
Ruby Iris Juric, Stephan Gerhold
In-Reply-To: <20240405-msm8916-moto-init-v1-0-502b58176d34@trvn.ru>
From: Ruby Iris Juric <ruby@srxl.me>
Motorola Moto G4 Play is an msm8916 based smartphone.
Supported features:
- eMMC and SD;
- Buttons;
- Touchscreen;
- USB;
- Fuel Gauge;
- Sound;
- Accelerometer.
msm8916 Moto devices share significant portion of the design so the
common parts are separated into a common dtsi.
Signed-off-by: Ruby Iris Juric <ruby@srxl.me>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Nikita: Split up to common dtsi]
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/msm8916-motorola-common.dtsi | 161 +++++++++++++++++++++
.../boot/dts/qcom/msm8916-motorola-harpia.dts | 147 +++++++++++++++++++
3 files changed, 309 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 7d40ec5e7d21..8d3fc7cb7a4d 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -31,6 +31,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-gplus-fl8005a.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-huawei-g7.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-longcheer-l8150.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-longcheer-l8910.dtb
+dtb-$(CONFIG_ARCH_QCOM) += msm8916-motorola-harpia.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-mtp.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-a3u-eur.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-a5u-eur.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-motorola-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-motorola-common.dtsi
new file mode 100644
index 000000000000..6a27d0ecd2ad
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-motorola-common.dtsi
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+ aliases {
+ mmc0 = &sdhc_1; /* eMMC */
+ mmc1 = &sdhc_2; /* SD card */
+ serial0 = &blsp_uart1;
+ };
+
+ chosen {
+ stdout-path = "serial0";
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ pinctrl-0 = <&gpio_keys_default>;
+ pinctrl-names = "default";
+
+ label = "GPIO Buttons";
+
+ volume-up-button {
+ label = "Volume Up";
+ gpios = <&tlmm 107 GPIO_ACTIVE_LOW>;
+ linux,code = <KEY_VOLUMEUP>;
+ debounce-interval = <15>;
+ };
+ };
+
+ usb_id: usb-id {
+ compatible = "linux,extcon-usb-gpio";
+ id-gpios = <&tlmm 91 GPIO_ACTIVE_HIGH>;
+ pinctrl-0 = <&usb_id_default>;
+ pinctrl-1 = <&usb_id_sleep>;
+ pinctrl-names = "default", "sleep";
+ };
+};
+
+&blsp_i2c2 {
+ status = "okay";
+
+ touchscreen: touchscreen@20 {
+ compatible = "syna,rmi4-i2c";
+ reg = <0x20>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vio-supply = <&pm8916_l6>;
+
+ syna,startup-delay-ms = <100>;
+
+ rmi4-f01@1 {
+ reg = <1>;
+ syna,nosleep-mode = <1>; /* Allow sleeping */
+ };
+
+ rmi4-f11@11 {
+ reg = <11>;
+ syna,sensor-type = <1>; /* Touchscreen */
+ };
+ };
+};
+
+&blsp_uart1 {
+ status = "okay";
+};
+
+&mpss_mem {
+ reg = <0x0 0x86800000 0x0 0x5500000>;
+};
+
+&pm8916_resin {
+ linux,code = <KEY_VOLUMEDOWN>;
+ status = "okay";
+};
+
+&pm8916_rpm_regulators {
+ pm8916_l16: l16 {
+ regulator-min-microvolt = <3100000>;
+ regulator-max-microvolt = <3300000>;
+ };
+};
+
+&pm8916_vib {
+ status = "okay";
+};
+
+&sdhc_1 {
+ status = "okay";
+};
+
+&sdhc_2 {
+ status = "okay";
+};
+
+&usb {
+ extcon = <&usb_id>, <&usb_id>;
+ status = "okay";
+};
+
+&usb_hs_phy {
+ extcon = <&usb_id>;
+};
+
+&venus {
+ status = "okay";
+};
+
+&venus_mem {
+ status = "okay";
+};
+
+&wcnss {
+ status = "okay";
+};
+
+&wcnss_iris {
+ compatible = "qcom,wcn3620";
+};
+
+&wcnss_mem {
+ status = "okay";
+};
+
+/* CTS/RTX are not used */
+&blsp_uart1_default {
+ pins = "gpio0", "gpio1";
+};
+&blsp_uart1_sleep {
+ pins = "gpio0", "gpio1";
+};
+
+&tlmm {
+ gpio_keys_default: gpio-keys-default-state {
+ pins = "gpio107";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ usb_id_default: usb-id-default-state {
+ pins = "gpio91";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-up;
+ };
+
+ usb_id_sleep: usb-id-sleep-state {
+ pins = "gpio91";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-disable;
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916-motorola-harpia.dts b/arch/arm64/boot/dts/qcom/msm8916-motorola-harpia.dts
new file mode 100644
index 000000000000..8380451ebbf6
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-motorola-harpia.dts
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-motorola-common.dtsi"
+
+/ {
+ model = "Motorola Moto G4 Play";
+ compatible = "motorola,harpia", "qcom,msm8916";
+ chassis-type = "handset";
+};
+
+&blsp_i2c1 {
+ status = "okay";
+
+ battery@36 {
+ compatible = "maxim,max17050";
+ reg = <0x36>;
+
+ interrupts-extended = <&tlmm 62 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-0 = <&battery_alert_default>;
+ pinctrl-names = "default";
+
+ maxim,rsns-microohm = <10000>;
+ maxim,over-heat-temp = <600>;
+ maxim,cold-temp = <(-200)>;
+ maxim,dead-volt = <3200>;
+ maxim,over-volt = <4500>;
+ };
+
+ /* charger@6b */
+};
+
+&blsp_i2c4 {
+ status = "okay";
+
+ accelerometer@19 {
+ compatible = "bosch,bma253";
+ reg = <0x19>;
+
+ interrupts-extended = <&tlmm 115 IRQ_TYPE_EDGE_RISING>,
+ <&tlmm 119 IRQ_TYPE_EDGE_RISING>;
+
+ vdd-supply = <&pm8916_l17>;
+ vddio-supply = <&pm8916_l6>;
+
+ mount-matrix = "1", "0", "0",
+ "0", "-1", "0",
+ "0", "0", "1";
+
+ pinctrl-0 = <&accel_int_default>;
+ pinctrl-names = "default";
+ };
+
+ /* proximity@49 */
+};
+
+&pm8916_codec {
+ qcom,micbias-lvl = <2800>;
+ qcom,mbhc-vthreshold-low = <75 150 237 450 500>;
+ qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
+ qcom,micbias1-ext-cap;
+};
+
+&pm8916_rpm_regulators {
+ pm8916_l17: l17 {
+ regulator-min-microvolt = <2850000>;
+ regulator-max-microvolt = <2850000>;
+ };
+};
+
+&sdhc_2 {
+ pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
+ pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
+ pinctrl-names = "default", "sleep";
+
+ cd-gpios = <&tlmm 118 GPIO_ACTIVE_LOW>;
+};
+
+&sound {
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+
+ pinctrl-0 = <&cdc_pdm_default &headset_switch_supply_en
+ &headset_switch_in>;
+ pinctrl-1 = <&cdc_pdm_sleep &headset_switch_supply_en
+ &headset_switch_in>;
+ pinctrl-names = "default", "sleep";
+};
+
+&touchscreen {
+ interrupts-extended = <&tlmm 13 IRQ_TYPE_EDGE_FALLING>;
+
+ vdd-supply = <&pm8916_l16>;
+
+ pinctrl-0 = <&ts_int_default>;
+ pinctrl-names = "default";
+};
+
+&tlmm {
+ accel_int_default: accel-int-default-state {
+ pins = "gpio115", "gpio119";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ battery_alert_default: battery-alert-default-state {
+ pins = "gpio62";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ headset_switch_in: headset-switch-in-state {
+ pins = "gpio112";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ output-low;
+ };
+
+ headset_switch_supply_en: headset-switch-supply-en-state {
+ pins = "gpio111";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ output-high;
+ };
+
+ sdc2_cd_default: sdc2-cd-default-state {
+ pins = "gpio118";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ ts_int_default: ts-int-default-state {
+ pins = "gpio13";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+};
--
2.44.0
^ permalink raw reply related
* [PATCH 3/4] arm64: dts: qcom: Add Motorola Moto E 2015 LTE (surnia)
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin,
Wiktor Strzębała, Valérie Roux, Stephan Gerhold
In-Reply-To: <20240405-msm8916-moto-init-v1-0-502b58176d34@trvn.ru>
From: Wiktor Strzębała <wiktorek140@gmail.com>
Motorola Moto E 2015 LTE is an msm8916 based smartphone.
Supported features:
- eMMC and SD;
- Buttons;
- Touchscreen;
- USB;
- Fuel Gauge;
- Sound.
Signed-off-by: Wiktor Strzębała <wiktorek140@gmail.com>
[Valérie: Sound and modem]
Co-developed-by: Valérie Roux <undev@unixgirl.xyz>
Signed-off-by: Valérie Roux <undev@unixgirl.xyz>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Nikita: Use common dtsi]
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/msm8916-motorola-surnia.dts | 83 ++++++++++++++++++++++
2 files changed, 84 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 8d3fc7cb7a4d..acf3ee316fba 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -32,6 +32,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-huawei-g7.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-longcheer-l8150.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-longcheer-l8910.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-motorola-harpia.dtb
+dtb-$(CONFIG_ARCH_QCOM) += msm8916-motorola-surnia.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-mtp.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-a3u-eur.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-a5u-eur.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-motorola-surnia.dts b/arch/arm64/boot/dts/qcom/msm8916-motorola-surnia.dts
new file mode 100644
index 000000000000..eecf78ba45bb
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-motorola-surnia.dts
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-motorola-common.dtsi"
+
+/ {
+ model = "Motorola Moto E 2015 LTE";
+ compatible = "motorola,surnia", "qcom,msm8916";
+ chassis-type = "handset";
+};
+
+&blsp_i2c4 {
+ status = "okay";
+
+ battery@36 {
+ compatible = "maxim,max17050";
+ reg = <0x36>;
+
+ interrupts-extended = <&tlmm 12 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-0 = <&battery_alert_default>;
+ pinctrl-names = "default";
+
+ maxim,rsns-microohm = <10000>;
+ maxim,over-heat-temp = <600>;
+ maxim,cold-temp = <(-200)>;
+ maxim,dead-volt = <3200>;
+ maxim,over-volt = <4500>;
+
+ };
+};
+
+&pm8916_codec {
+ qcom,micbias1-ext-cap;
+ qcom,micbias2-ext-cap;
+};
+
+&sdhc_2 {
+ pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
+ pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
+ pinctrl-names = "default", "sleep";
+
+ cd-gpios = <&tlmm 25 GPIO_ACTIVE_LOW>;
+};
+
+&sound {
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC3", "MIC BIAS External1";
+};
+
+&touchscreen {
+ interrupts-extended = <&tlmm 21 IRQ_TYPE_EDGE_FALLING>;
+
+ vdd-supply = <&pm8916_l16>;
+
+ pinctrl-0 = <&ts_int_default>;
+ pinctrl-names = "default";
+};
+
+&tlmm {
+ battery_alert_default: battery-alert-default-state {
+ pins = "gpio12";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ sdc2_cd_default: sdc2-cd-default-state {
+ pins = "gpio25";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ ts_int_default: ts-int-default-state {
+ pins = "gpio21";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+};
--
2.44.0
^ permalink raw reply related
* [PATCH 4/4] arm64: dts: qcom: Add Motorola Moto G 2015 (osprey)
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin,
Martijn Braam, Stephan Gerhold
In-Reply-To: <20240405-msm8916-moto-init-v1-0-502b58176d34@trvn.ru>
From: Martijn Braam <martijn@brixit.nl>
Motorola Moto G 2015 is an msm8916 based smartphone.
Supported features:
- eMMC and SD;
- Buttons;
- Touchscreen;
- USB;
- Fuel Gauge;
- Sound.
Signed-off-by: Martijn Braam <martijn@brixit.nl>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Nikita: Use common dtsi]
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/msm8916-motorola-osprey.dts | 105 +++++++++++++++++++++
2 files changed, 106 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index acf3ee316fba..ff23afb1b0db 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -32,6 +32,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-huawei-g7.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-longcheer-l8150.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-longcheer-l8910.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-motorola-harpia.dtb
+dtb-$(CONFIG_ARCH_QCOM) += msm8916-motorola-osprey.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-motorola-surnia.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-mtp.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-a3u-eur.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-motorola-osprey.dts b/arch/arm64/boot/dts/qcom/msm8916-motorola-osprey.dts
new file mode 100644
index 000000000000..ec5589fc69bd
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-motorola-osprey.dts
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-motorola-common.dtsi"
+
+/ {
+ model = "Motorola Moto G 2015";
+ compatible = "motorola,osprey", "qcom,msm8916";
+ chassis-type = "handset";
+
+ reg_touch_vdda: regulator-touch-vdda {
+ compatible = "regulator-fixed";
+ regulator-name = "touch_vdda";
+ gpio = <&tlmm 114 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ pinctrl-0 = <&touch_vdda_default>;
+ pinctrl-names = "default";
+ startup-delay-us = <300>;
+ vin-supply = <&pm8916_l16>;
+ };
+};
+
+&blsp_i2c1 {
+ status = "okay";
+
+ battery@36 {
+ compatible = "maxim,max17050";
+ reg = <0x36>;
+
+ interrupts-extended = <&tlmm 49 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-0 = <&battery_alert_default>;
+ pinctrl-names = "default";
+
+ maxim,rsns-microohm = <10000>;
+ maxim,over-heat-temp = <600>;
+ maxim,cold-temp = <(-200)>;
+ maxim,dead-volt = <3200>;
+ maxim,over-volt = <4500>;
+
+ };
+};
+
+&blsp_i2c6 {
+ /* magnetometer@c */
+};
+
+&pm8916_codec {
+ qcom,micbias1-ext-cap;
+ qcom,micbias2-ext-cap;
+};
+
+&sdhc_2 {
+ pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
+ pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
+ pinctrl-names = "default", "sleep";
+
+ cd-gpios = <&tlmm 25 GPIO_ACTIVE_LOW>;
+};
+
+&sound {
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC3", "MIC BIAS External1";
+};
+
+&touchscreen {
+ interrupts-extended = <&tlmm 21 IRQ_TYPE_EDGE_FALLING>;
+
+ vdd-supply = <®_touch_vdda>;
+
+ pinctrl-0 = <&ts_int_default>;
+ pinctrl-names = "default";
+};
+
+&tlmm {
+ battery_alert_default: battery-alert-default-state {
+ pins = "gpio49";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ sdc2_cd_default: sdc2-cd-default-state {
+ pins = "gpio25";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ ts_int_default: ts-int-default-state {
+ pins = "gpio21";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ touch_vdda_default: touch-vdda-default-state {
+ pins = "gpio114";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+};
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v8 1/4] ASoc: PCM6240: Create PCM6240 Family driver code
From: Mark Brown @ 2024-04-05 14:09 UTC (permalink / raw)
To: Shenghao Ding
Cc: linux-kernel, lgirdwood, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-sound, devicetree, perex, tiwai, 13916275206,
mohit.chawla, soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal,
aviel, hnagalla, praneeth, Baojun.Xu
In-Reply-To: <20240403003159.389-2-shenghao-ding@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]
On Wed, Apr 03, 2024 at 08:31:55AM +0800, Shenghao Ding wrote:
> +static const char *const pcmdev_ctrl_name[] = {
> + "%s-i2c-%d-dev%d-ch%d-ana-gain",
> + "%s-i2c-%d-dev%d-ch%d-digi-gain",
> + "%s-i2c-%d-dev%d-ch%d-fine-gain",
> +};
> +
> +static const char *const pcmdev_ctrl_name_with_prefix[] = {
> + "%s-dev%d-ch%d-ana-gain",
> + "%s-dev%d-ch%d-digi-gain",
> + "%s-dev%d-ch%d-fine-gain",
> +};
These still don't look like idiomatic ALSA control names, if nothing
else volume controls should end with Volume but the whole all lower case
with -s thing really isn't idiomatic.
> +static int pcmdev_put_volsw(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol, int vol_ctrl_type)
> +{
> + err = pcmdev_dev_update_bits(pcm_dev, dev_no, reg, val_mask, val);
> + if (err) {
> + dev_err(pcm_dev->dev, "%s: update_bits err = %d\n",
> + __func__, err);
> + err = 0;
> + }
Why don't we report errors to users?
> + case PCMDEVICE_CMD_DELAY: {
> + unsigned int delay_time = 0;
> +
> + if (subblk_offset + 2 > sublocksize) {
> + dev_err(pcm_dev->dev,
> + "%s: deley out of boundary\n",
> + __func__);
> + break;
> + }
> + delay_time = get_unaligned_be16(&data[2]) * 1000;
> + usleep_range(delay_time, delay_time + 50);
> + subblk_offset += 2;
> + }
> + break;
> + case PCMDEVICE_CMD_FIELD_W:
> + if (subblk_offset + 6 > sublocksize) {
> + dev_err(pcm_dev->dev,
> + "%s: bit write out of memory\n", __func__);
> + break;
> + }
> + ret = pcmdev_dev_update_bits(pcm_dev, chn,
> + PCMDEVICE_REG(data[subblk_offset + 3],
> + data[subblk_offset + 4]), data[subblk_offset + 1],
> + data[subblk_offset + 5]);
The indentation here is all messed up, things not indented for the case
blocks and so on.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RESEND v7 21/37] dt-bindings: serial: renesas,scif: Add scif-sh7751.
From: Geert Uytterhoeven @ 2024-04-05 14:02 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev, Lad, Prabhakar
In-Reply-To: <f3af315d575fbec431bad9bfaf9790450ab31ad9.1712207606.git.ysato@users.sourceforge.jp>
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Add Renesas SH7751 SCIF.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> @@ -18,6 +18,7 @@ properties:
> - items:
> - enum:
> - renesas,scif-r7s72100 # RZ/A1H
> + - renesas,scif-sh7751 # SH7751
> - const: renesas,scif # generic SCIF compatible UART
>
> - items:
If this is applied after "[PATCH v2 2/2] dt-bindings: serial:
renesas,scif: Validate 'interrupts' and 'interrupt-names'"[1], an extra
"- renesas,scif-sh7751" line should be added to the 4-interrupt section
(below "- renesas,scif-r7s72100").
[1] https://lore.kernel.org/all/20240307114217.34784-3-prabhakar.mahadev-lad.rj@bp.renesas.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 03/18] PCI: endpoint: Introduce pci_epc_mem_map()/unmap()
From: Niklas Cassel @ 2024-04-05 14:10 UTC (permalink / raw)
To: Damien Le Moal
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240330041928.1555578-4-dlemoal@kernel.org>
On Sat, Mar 30, 2024 at 01:19:13PM +0900, Damien Le Moal wrote:
> Introduce the function pci_epc_mem_map() to facilitate controller memory
> address allocation and mapping to a RC PCI address region in endpoint
> function drivers.
>
> This function first uses pci_epc_map_align() to determine the controller
> memory address alignment (offset and size) constraints. The result of
> this function is used to allocate a controller physical memory region
> using pci_epc_mem_alloc_addr() and map it to the RC PCI address
> space with pci_epc_map_addr(). Since pci_epc_map_align() may indicate
> that a mapping can be smaller than the requested size, pci_epc_mem_map()
> may only partially map the RC PCI address region specified and return
> a smaller size for the effective mapping.
>
> The counterpart of pci_epc_mem_map() to unmap and free the controller
> memory address region is pci_epc_mem_unmap().
>
> Both functions operate using struct pci_epc_map data structure which is
> extended to contain the physical and virtual addresses of the allocated
> controller memory. Endpoint function drivers can use struct pci_epc_map
> to implement read/write accesses within the mapped RC PCI address region
> using the ->virt_addr and ->size fields.
>
> This commit contains contributions from Rick Wertenbroek
> <rick.wertenbroek@gmail.com>.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> drivers/pci/endpoint/pci-epc-core.c | 68 +++++++++++++++++++++++++++++
> include/linux/pci-epc.h | 6 +++
> 2 files changed, 74 insertions(+)
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 37758ca91d7f..0095b54bdf9e 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -530,6 +530,74 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> }
> EXPORT_SYMBOL_GPL(pci_epc_map_addr);
>
> +/**
> + * pci_epc_mem_map() - allocate and map CPU address to PCI address
> + * @epc: the EPC device on which the CPU address is to be allocated and mapped
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @pci_addr: PCI address to which the CPU address should be mapped
> + * @size: the number of bytes to map starting from @pci_addr
> + * @map: where to return the mapping information
> + *
> + * Allocate a controller physical address region and map it to a RC PCI address
> + * region, taking into account the controller physical address mapping
> + * constraints (if any). Returns the effective size of the mapping, which may
> + * be less than @size, or a negative error code in case of error.
> + */
> +ssize_t pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + u64 pci_addr, size_t size, struct pci_epc_map *map)
> +{
> + int ret;
> +
> + ret = pci_epc_map_align(epc, func_no, vfunc_no, pci_addr, size, map);
> + if (ret)
> + return ret;
> +
> + map->virt_base = pci_epc_mem_alloc_addr(epc, &map->phys_base,
> + map->map_size);
> + if (!map->virt_base)
> + return -ENOMEM;
> +
> + map->phys_addr = map->phys_base + map->map_ofst;
> + map->virt_addr = map->virt_base + map->map_ofst;
> +
> + ret = pci_epc_map_addr(epc, func_no, vfunc_no, map->phys_base,
> + map->map_pci_addr, map->map_size);
> + if (ret) {
> + pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
> + map->map_size);
> + return ret;
> + }
> +
> + return map->pci_size;
map->pci_size is of type size_t.
pci_epc_mem_map returns a type of ssize_t.
This means that on ILP32 you will truncate the result, and will only be
able to map a region of max size 2GB.
Could we perhaps change this function to return an int instead?
(0 on success). The mapped size can still be accessed in map->pci_size.
Kind regards,
Niklas
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_mem_map);
> +
> +/**
> + * pci_epc_mem_unmap() - unmap from PCI address and free a CPU address region
> + * @epc: the EPC device on which the CPU address is allocated and mapped
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @map: the mapping information
> + *
> + * Allocate and map local CPU address to a PCI address, accounting for the
> + * controller local CPU address alignment constraints.
> + */
> +void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + struct pci_epc_map *map)
> +{
> + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> + return;
> +
> + if (!map || !map->pci_size)
> + return;
> +
> + pci_epc_unmap_addr(epc, func_no, vfunc_no, map->phys_base);
> + pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
> + map->map_size);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_mem_unmap);
> +
> /**
> * pci_epc_clear_bar() - reset the BAR
> * @epc: the EPC device for which the BAR has to be cleared
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 8cfb4aaf2628..86397a500b54 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -304,4 +304,10 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size);
> void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
> void __iomem *virt_addr, size_t size);
> +
> +ssize_t pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + u64 pci_addr, size_t size, struct pci_epc_map *map);
> +void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + struct pci_epc_map *map);
> +
> #endif /* __LINUX_PCI_EPC_H */
> --
> 2.44.0
>
^ permalink raw reply
* Re: [PATCH v3 21/25] drivers: media: i2c: imx258: Use macros
From: Tommaso Merciai @ 2024-04-05 14:11 UTC (permalink / raw)
To: Luis Garcia
Cc: Sakari Ailus, linux-media, dave.stevenson, jacopo.mondi, mchehab,
robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel, Ondrej Jirman
In-Reply-To: <082190a8-7ac5-4240-9a16-6b9168c67d57@luigi311.com>
Hi Luis,
On Fri, Apr 05, 2024 at 04:33:38AM -0600, Luis Garcia wrote:
> On 4/4/24 00:46, Sakari Ailus wrote:
> > On Wed, Apr 03, 2024 at 01:17:26PM -0600, Luigi311 wrote:
> >> On 4/3/24 10:23, Sakari Ailus wrote:
> >>> Hi Luis,
> >>>
> >>> On Wed, Apr 03, 2024 at 09:03:50AM -0600, git@luigi311.com wrote:
> >>>> From: Luis Garcia <git@luigi311.com>
> >>>>
> >>>> Use understandable macros instead of raw values.
> >>>>
> >>>> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> >>>> Signed-off-by: Luis Garcia <git@luigi311.com>
> >>>> ---
> >>>> drivers/media/i2c/imx258.c | 434 ++++++++++++++++++-------------------
> >>>> 1 file changed, 207 insertions(+), 227 deletions(-)
> >>>>
> >>>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> >>>> index e2ecf6109516..30352c33f63c 100644
> >>>> --- a/drivers/media/i2c/imx258.c
> >>>> +++ b/drivers/media/i2c/imx258.c
> >>>> @@ -33,8 +33,6 @@
> >>>> #define IMX258_VTS_30FPS_VGA 0x034c
> >>>> #define IMX258_VTS_MAX 65525
> >>>>
> >>>> -#define IMX258_REG_VTS 0x0340
> >>>> -
> >>>> /* HBLANK control - read only */
> >>>> #define IMX258_PPL_DEFAULT 5352
> >>>>
> >>>> @@ -90,6 +88,53 @@
> >>>> #define IMX258_PIXEL_ARRAY_WIDTH 4208U
> >>>> #define IMX258_PIXEL_ARRAY_HEIGHT 3120U
> >>>>
> >>>> +/* regs */
> >>>> +#define IMX258_REG_PLL_MULT_DRIV 0x0310
> >>>> +#define IMX258_REG_IVTPXCK_DIV 0x0301
> >>>> +#define IMX258_REG_IVTSYCK_DIV 0x0303
> >>>> +#define IMX258_REG_PREPLLCK_VT_DIV 0x0305
> >>>> +#define IMX258_REG_IOPPXCK_DIV 0x0309
> >>>> +#define IMX258_REG_IOPSYCK_DIV 0x030b
> >>>> +#define IMX258_REG_PREPLLCK_OP_DIV 0x030d
> >>>> +#define IMX258_REG_PHASE_PIX_OUTEN 0x3030
> >>>> +#define IMX258_REG_PDPIX_DATA_RATE 0x3032
> >>>> +#define IMX258_REG_SCALE_MODE 0x0401
> >>>> +#define IMX258_REG_SCALE_MODE_EXT 0x3038
> >>>> +#define IMX258_REG_AF_WINDOW_MODE 0x7bcd
> >>>> +#define IMX258_REG_FRM_LENGTH_CTL 0x0350
> >>>> +#define IMX258_REG_CSI_LANE_MODE 0x0114
> >>>> +#define IMX258_REG_X_EVN_INC 0x0381
> >>>> +#define IMX258_REG_X_ODD_INC 0x0383
> >>>> +#define IMX258_REG_Y_EVN_INC 0x0385
> >>>> +#define IMX258_REG_Y_ODD_INC 0x0387
> >>>> +#define IMX258_REG_BINNING_MODE 0x0900
> >>>> +#define IMX258_REG_BINNING_TYPE_V 0x0901
> >>>> +#define IMX258_REG_FORCE_FD_SUM 0x300d
> >>>> +#define IMX258_REG_DIG_CROP_X_OFFSET 0x0408
> >>>> +#define IMX258_REG_DIG_CROP_Y_OFFSET 0x040a
> >>>> +#define IMX258_REG_DIG_CROP_IMAGE_WIDTH 0x040c
> >>>> +#define IMX258_REG_DIG_CROP_IMAGE_HEIGHT 0x040e
> >>>> +#define IMX258_REG_SCALE_M 0x0404
> >>>> +#define IMX258_REG_X_OUT_SIZE 0x034c
> >>>> +#define IMX258_REG_Y_OUT_SIZE 0x034e
> >>>> +#define IMX258_REG_X_ADD_STA 0x0344
> >>>> +#define IMX258_REG_Y_ADD_STA 0x0346
> >>>> +#define IMX258_REG_X_ADD_END 0x0348
> >>>> +#define IMX258_REG_Y_ADD_END 0x034a
> >>>> +#define IMX258_REG_EXCK_FREQ 0x0136
> >>>> +#define IMX258_REG_CSI_DT_FMT 0x0112
> >>>> +#define IMX258_REG_LINE_LENGTH_PCK 0x0342
> >>>> +#define IMX258_REG_SCALE_M_EXT 0x303a
> >>>> +#define IMX258_REG_FRM_LENGTH_LINES 0x0340
> >>>> +#define IMX258_REG_FINE_INTEG_TIME 0x0200
> >>>> +#define IMX258_REG_PLL_IVT_MPY 0x0306
> >>>> +#define IMX258_REG_PLL_IOP_MPY 0x030e
> >>>> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H 0x0820
> >>>> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L 0x0822
> >>>> +
> >>>> +#define REG8(a, v) { a, v }
> >>>> +#define REG16(a, v) { a, ((v) >> 8) & 0xff }, { (a) + 1, (v) & 0xff }
> >>>
> >>> The patch is nice but these macros are better replaced by the V4L2 CCI
> >>> helper that also offers register access functions. Could you add a patch to
> >>> convert the driver to use it (maybe after this one)?
> >>>
> >>
> >> Ohh perfect, using something else would be great. Ill go ahead and see
> >> if I can get that working.
> >
> > Thanks. It may be easier to just do it in this one actually. Up to you.
> >
>
> I've made the swap but looks like its not playing nice with my ppp,
> its causing a crash and showing a call trace as soon as it does its
> first read to check the identity. I went in and dropped the cci_read
> and left it with the original implementation and I'm getting a very
> similar crash with cci_write too so it looks like its not liking
> how I'm implementing it. Looking at the few other drivers that were
> swapped over to use that, I don't seem to be missing anything. It's
> a big change so its not really something I can describe what I've
> changed but I do have the change on my github here
> https://github.com/luigi311/linux/commit/840593acb20eee87ce361e6929edf51eefbbe737
I checked your commit to switch to cci helper.
I think you are missing the right cci regmap initialization.
Please take care to use: devm_cci_regmap_init_i2c
/**
* devm_cci_regmap_init_i2c() - Create regmap to use with cci_*() register
* access functions
*
* @client: i2c_client to create the regmap for
* @reg_addr_bits: register address width to use (8 or 16)
*
* Note the memory for the created regmap is devm() managed, tied to the client.
*
* Return: %0 on success or a negative error code on failure.
*/
Check drivers/media/i2c/imx290.c:1530
Hope this help :)
Note:
Somewhere into the github commit you are reading 16bit reg and storing
that into 64bit val. Take care! :)
Thanks & Regards,
Tommaso
> if you can provide some guidance, if not I can skip this change
> all together and we can do a separate attempt at swapping over to it.
>
^ permalink raw reply
* Re: [PATCH v4 5/7] PCI: dwc: rcar-gen4: Add .ltssm_enable() for other SoC support
From: Geert Uytterhoeven @ 2024-04-05 14:13 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, kw, robh, bhelgaas, krzysztof.kozlowski+dt, conor+dt,
jingoohan1, mani, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc
In-Reply-To: <20240403053304.3695096-6-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Wed, Apr 3, 2024 at 7:33 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This driver can reuse other R-Car Gen4 SoCs support like r8a779g0 and
> r8a779h0. However, r8a779g0 and r8a779h0 require other initializing
> settings that differ than r8a779f0. So, add a new function pointer
> .ltssm_enable() for it. No behavior changes.
>
> After applied this patch, probing SoCs by rcar_gen4_pcie_of_match[]
> will be changed like below:
>
> - r8a779f0 as "renesas,r8a779f0-pcie" and "renesas,r8a779f0-pcie-ep"
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Thanks for your patch!
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -513,6 +536,14 @@ static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie_ep = {
> };
>
> static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> + {
> + .compatible = "renesas,r8a779f0-pcie",
> + .data = &platdata_r8a779f0_pcie,
> + },
> + {
> + .compatible = "renesas,r8a779f04-pcie-ep",
renesas,r8a779f0-pcie-ep
> + .data = &platdata_r8a779f0_pcie_ep,
> + },
> {
> .compatible = "renesas,rcar-gen4-pcie",
> .data = &platdata_rcar_gen4_pcie,
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v15 2/8] phy: Add HDMI configuration options
From: Vinod Koul @ 2024-04-05 14:24 UTC (permalink / raw)
To: Maxime Ripard
Cc: Alexander Stein, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
Daniel Vetter, Maarten Lankhorst, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kishon Vijay Abraham I,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sandor Yu, dri-devel, devicetree, linux-kernel, linux-phy, imx,
linux-arm-kernel, linux, Dmitry Baryshkov
In-Reply-To: <20240306-inquisitive-funny-bull-017550@houat>
On 06-03-24, 15:48, Maxime Ripard wrote:
> Hi Alexander,
>
> On Wed, Mar 06, 2024 at 11:16:19AM +0100, Alexander Stein wrote:
> > From: Sandor Yu <Sandor.yu@nxp.com>
> >
> > Allow HDMI PHYs to be configured through the generic
> > functions through a custom structure added to the generic union.
> >
> > The parameters added here are based on HDMI PHY
> > implementation practices. The current set of parameters
> > should cover the potential users.
> >
> > Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Acked-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > include/linux/phy/phy-hdmi.h | 24 ++++++++++++++++++++++++
> > include/linux/phy/phy.h | 7 ++++++-
> > 2 files changed, 30 insertions(+), 1 deletion(-)
> > create mode 100644 include/linux/phy/phy-hdmi.h
> >
> > diff --git a/include/linux/phy/phy-hdmi.h b/include/linux/phy/phy-hdmi.h
> > new file mode 100644
> > index 0000000000000..b7de88e9090f0
> > --- /dev/null
> > +++ b/include/linux/phy/phy-hdmi.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright 2022 NXP
> > + */
> > +
> > +#ifndef __PHY_HDMI_H_
> > +#define __PHY_HDMI_H_
> > +
> > +#include <linux/hdmi.h>
> > +/**
> > + * struct phy_configure_opts_hdmi - HDMI configuration set
> > + * @pixel_clk_rate: Pixel clock of video modes in KHz.
> > + * @bpc: Maximum bits per color channel.
> > + * @color_space: Colorspace in enum hdmi_colorspace.
> > + *
> > + * This structure is used to represent the configuration state of a HDMI phy.
> > + */
> > +struct phy_configure_opts_hdmi {
> > + unsigned int pixel_clk_rate;
> > + unsigned int bpc;
> > + enum hdmi_colorspace color_space;
> > +};
>
> Does the PHY actually care about the pixel clock rate, color space and
> formats, or does it only care about the character rate?
Nope it should not
--
~Vinod
^ permalink raw reply
* Re: [RESEND v7 13/37] dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header.
From: Geert Uytterhoeven @ 2024-04-05 12:40 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <1db8627e4ca50b9d2d27e95d245518cac1cd62dc.1712207606.git.ysato@users.sourceforge.jp>
Hi Sato-san,
Thanks for the update!
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
>
> SH7750 CPG Clock output define.
(from my comments on v6) Please improve the patch description.
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> index 000000000000..04c10b0834ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
The actual bindings LGTM.
> +examples:
> + - |
> + #include <dt-bindings/clock/sh7750-cpg.h>
> + cpg: clock-controller@ffc00000 {
> + #clock-cells = <1>;
> + #power-domain-cells = <0>;
> + compatible = "renesas,sh7751r-cpg";
> + clocks = <&extal>;
> + clock-names = "extal";
> + reg = <0xffc00000 20>, <0xfe0a0000 16>;
> + reg-names = "FRQCR", "CLKSTP00";
> + renesas,mode = <0>;
> + };
Please read
https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 0/2] Enable JPEG encoding on rk3588
From: Link Mauve @ 2024-04-05 14:21 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Emmanuel Gil Peyrot, linux-kernel, Ezequiel Garcia, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
Robin Murphy, Sebastian Reichel, Cristian Ciocaltea, Dragan Simic,
Shreeya Patel, Chris Morgan, Andy Yan, Nicolas Frattaroli,
linux-media, linux-rockchip, devicetree, linux-arm-kernel, iommu
In-Reply-To: <bbcb66e9499120a86b367e7abdac2d8e2e704bfb.camel@ndufresne.ca>
On Thu, Apr 04, 2024 at 01:41:15PM -0400, Nicolas Dufresne wrote:
> Hi,
Hi,
>
> Le mercredi 27 mars 2024 à 14:41 +0100, Emmanuel Gil Peyrot a écrit :
> > Only the JPEG encoder is available for now, although there are patches
> > for the undocumented VP8 encoder floating around[0].
>
> [0] seems like a broken link. The VP8 encoder RFC is for RK3399 (and Hantro H1
> posted by ST more recently). The TRM says "VEPU121(JPEG encoder only)", which
> suggest that the H.264 and VP8 encoders usually found on the VEPU121 are
> removed. As Rockchip have remove the synthesize register while modifying the H1
> IP, it is difficult to verify. Confusingly the H.264 specific registers are
> documented in the TRM around VEPU121.
Ah, the link became, and was indeed ST’s series:
https://patchwork.kernel.org/project/linux-rockchip/list/?series=789885&archive=both
But the TRM part 1 says the VEPU121 supports H.264 encoding (page 367),
and it’s likely they didn’t remove just VP8 support since the codec
features are pretty close to H.264’s.
>
> >
> > This has been tested on a rock-5b, resulting in four /dev/video*
> > encoders. The userspace program I’ve been using to test them is
> > Onix[1], using the jpeg-encoder example, it will pick one of these four
> > at random (but displays the one it picked):
> > % ffmpeg -i <input image> -pix_fmt yuvj420p temp.yuv
> > % jpeg-encoder temp.yuv <width> <height> NV12 <quality> output.jpeg
>
> I don't like that we exposing each identical cores a separate video nodes. I
> think we should aim for 1 device, and then multi-plex and schedule de cores from
> inside the Linux kernel.
I agree, but this should be handled in the driver not in the device
tree, and it can be done later.
>
> Not doing this now means we'll never have an optimal hardware usage
> distribution. Just consider two userspace software wanting to do jpeg encoding.
> If they both take a guess, they may endup using a single core. Where with proper
> scheduling in V4L2, the kernel will be able to properly distribute the load. I
> insist on this, since if we merge you changes it becomes an ABI and we can't
> change it anymore.
Will it really become ABI just like that? Userspace should always
discover the video nodes and their capabilities and not hardcode e.g. a
specific /dev/videoN file for a specific codec. I would argue that this
series would let userspace do JPEG encoding right away, even if in a
less optimal way than if the driver would round-robin them through a
single video node, but that can always be added in a future version.
>
> I understand that this impose a rework of the mem2mem framework so that we can
> run multiple jobs, but this will be needed anyway on RK3588, since the rkvdec2,
> which we don't have a driver yet is also multi-core, but you need to use 2 cores
> when the resolution is close to 8K.
I think the mediatek JPEG driver already supports that, would it be ok
to do it the same way?
>
> Nicolas
>
> >
> > [0] https://patchwork.kernel.org/project/linux-rockchip/list/?series=789885
> > [1] https://crates.io/crates/onix
> >
> > Changes since v1:
> > - Dropped patches 1 and 4.
> > - Use the proper compatible form, since this device should be fully
> > compatible with the VEPU of rk356x.
> > - Describe where the VEPU121 name comes from, and list other encoders
> > and decoders present in this SoC.
> > - Properly test the device tree changes, I previously couldn’t since I
> > was using a too recent version of python-jsonschema…
> >
> > Emmanuel Gil Peyrot (2):
> > media: dt-binding: media: Document rk3588’s VEPU121
> > arm64: dts: rockchip: Add VEPU121 to rk3588
> >
> > .../bindings/media/rockchip,rk3568-vepu.yaml | 8 +-
> > arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 80 +++++++++++++++++++
> > 2 files changed, 86 insertions(+), 2 deletions(-)
> >
>
--
Link Mauve
^ 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