* Re: [PATCH v5 02/10] dt-bindings: mailbox: Add mboxes property for CMDQ secure driver
From: Conor Dooley @ 2024-04-05 16:13 UTC (permalink / raw)
To: Jason-JH Lin (林睿祥)
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
Houlong Wei (魏厚龙),
devicetree@vger.kernel.org, Shawn Sung (宋孝謙),
CK Hu (胡俊光), conor+dt@kernel.org,
robh@kernel.org, linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
jassisinghbrar@gmail.com, angelogioacchino.delregno@collabora.com
In-Reply-To: <e6a30feb1e4bb41c90df5e0272385d0f47a7dcab.camel@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 4393 bytes --]
On Fri, Apr 05, 2024 at 02:33:14PM +0000, Jason-JH Lin (林睿祥) wrote:
> On Thu, 2024-04-04 at 15:52 +0100, Conor Dooley wrote:
> > On Thu, Apr 04, 2024 at 04:31:06AM +0000, Jason-JH Lin (林睿祥) wrote:
> > > Hi Conor,
> > >
> > > Thanks for the reviews.
> > >
> > > On Wed, 2024-04-03 at 16:46 +0100, Conor Dooley wrote:
> > > > On Wed, Apr 03, 2024 at 06:25:54PM +0800, Shawn Sung wrote:
> > > > > From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> > > > >
> > > > > Add mboxes to define a GCE loopping thread as a secure irq
> > > > > handler.
> > > > > This property is only required if CMDQ secure driver is
> > > > > supported.
> > > > >
> > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > > > > ---
> > > > > .../bindings/mailbox/mediatek,gce-mailbox.yaml | 10
> > > > > ++++++++++
> > > > > 1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > index cef9d76013985..c0d80cc770899 100644
> > > > > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > @@ -49,6 +49,16 @@ properties:
> > > > > items:
> > > > > - const: gce
> > > > >
> > > > > + mediatek,gce-events:
> > > > > + description:
> > > > > + The event id which is mapping to the specific hardware
> > > > > event
> > > > > signal
> > > > > + to gce. The event id is defined in the gce header
> > > > > + include/dt-bindings/gce/<chip>-gce.h of each chips.
> > > >
> > > > Missing any info here about when this should be used, hint - you
> > > > have
> > > > it
> > > > in the commit message.
> > > >
> > > > > + $ref: /schemas/types.yaml#/definitions/uint32-arrayi
> > > >
> > > > Why is the ID used by the CMDQ service not fixed for each SoC?
> > > >
> > >
> > > I forgot to sync with Shawn about this:
> > > https://lore.kernel.org/all/20240124011459.12204-1-jason-
> > > jh.lin@mediatek.com
> > >
> > > I'll fix it at the next version.
> >
> > When I say "fixed" I don't mean "this is wrong, please fix it", I
> > mean
> > "why is the value not static for a particular SoC". This needs to be
> > explained in the patch (and the description for the event here needs
> > to
> > explain what the gce-mailbox is reserving an event for).
> >
> Oh, I see. Thanks for noticing me.
>
> We do want to reserve a static event ID for gce-mailbox to different
> SoCs. There are 2 mainly reasons to why we set it in DTS:
> 1. There are 1024 events IDs for GCE to use to execute instructions in
> the specific event happened. These events could be signaled by HW or SW
> and their value would be different in different SoC because of HW event
> IDs distribution range from 0 to 1023.
> If we set a static event ID: 855 for mt8188, it might be conflict the
> event ID original set in mt8195.
That's not a problem, we have compatibles for this purpose.
> 2. If we defined the event ID in DTS, we might know how many SW or HW
> event IDs are used.
> If someone wants to use a new event ID for a new feature, they could
> find out the used event IDs in DTS easily and avoid the event ID
> conflicting.
Are the event IDs not documented in the reference manual for the SoC in
question? Or in documentation for the secure world for these devices? A
DTS should not be the authoritive source for this information for
developers.
Additionally, the driver could very easily detect if someone does happen
to put in the reserved ID. That could be generically useful (IOW, check
all of them for re-use) if the ID are to not allowed to be shared.
> The reason why we define a event ID is we want to get a SW signal from
> secure world. We design a GCE looping thread in gce-mailbox driver to
> wait for the GCE execute done event for each cmdq secure packets from
> secure world.
This sort of information needs to be in the commit message, but I don't
think this property is needed at all since it seems to be something
detectable from the compatible.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v5 2/2] iio: adc: adding support for PAC193x
From: Conor Dooley @ 2024-04-05 16:14 UTC (permalink / raw)
To: Marius.Cristea
Cc: jic23, Conor.Dooley, linux, jdelvare, linux-iio, devicetree, lars,
linux-kernel, linux-hwmon, krzysztof.kozlowski+dt, robh+dt,
conor+dt
In-Reply-To: <e432bddc16952d9144ccf6da5a54b88e3171b947.camel@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 226 bytes --]
On Fri, Apr 05, 2024 at 12:53:38PM +0000, Marius.Cristea@microchip.com wrote:
> Hi Conor,
>
> Thanks for reporting the bug. I have detect it and I'm already
> working on a patch for it.
Oh cool, keep me posted :)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 18/25] dt-bindings: media: imx258: Add alternate compatible strings
From: Conor Dooley @ 2024-04-05 16:24 UTC (permalink / raw)
To: Dave Stevenson
Cc: git, linux-media, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, pavel, phone-devel
In-Reply-To: <CAPY8ntC9SHJ6Ma17s0Vf2coB-0NUk-xgCLK9KCkxFMuXKHXNwg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2440 bytes --]
On Fri, Apr 05, 2024 at 11:25:50AM +0100, Dave Stevenson wrote:
> Hi Conor
>
> On Wed, 3 Apr 2024 at 17:14, Conor Dooley <conor@kernel.org> wrote:
> >
> > On Wed, Apr 03, 2024 at 09:03:47AM -0600, git@luigi311.com wrote:
> > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > >
> > > There are a number of variants of the imx258 modules that can not
> > > be differentiated at runtime, so add compatible strings for the
> > > PDAF variant.
> > >
> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > Signed-off-by: Luis Garcia <git@luigi311.com>
> > > ---
> > > .../devicetree/bindings/media/i2c/sony,imx258.yaml | 9 +++++++--
> > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > index bee61a443b23..c978abc0cdb3 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > @@ -13,11 +13,16 @@ description: |-
> > > IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> > > type stacked image sensor with a square pixel array of size 4208 x 3120. It
> > > is programmable through I2C interface. Image data is sent through MIPI
> > > - CSI-2.
> > > + CSI-2. The sensor exists in two different models, a standard variant
> > > + (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
> > > + The camera module does not expose the model through registers, so the
> > > + exact model needs to be specified.
> > >
> > > properties:
> > > compatible:
> > > - const: sony,imx258
> > > + enum:
> > > + - sony,imx258
> > > + - sony,imx258-pdaf
> >
> > Does the pdaf variant support all of the features/is it register
> > compatible with the regular variant? If it is, the regular variant
> > should be a fallback compatible.
>
> It has the same register set, but certain registers have to be
> programmed differently so that the image is corrected for the
> partially shielded pixels used for phase detect auto focus (PDAF).
> Either compatible will "work" on either variant of the module, but
> you'll get weird image artifacts when using the wrong one.
To paraphase, a fallback compatible is not suitable.
Thanks Dave,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 18/25] dt-bindings: media: imx258: Add alternate compatible strings
From: Conor Dooley @ 2024-04-05 16:24 UTC (permalink / raw)
To: Dave Stevenson
Cc: git, linux-media, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, pavel, phone-devel
In-Reply-To: <20240405-affair-cruelly-a7e9d23b597c@spud>
[-- Attachment #1: Type: text/plain, Size: 2677 bytes --]
On Fri, Apr 05, 2024 at 05:24:11PM +0100, Conor Dooley wrote:
> On Fri, Apr 05, 2024 at 11:25:50AM +0100, Dave Stevenson wrote:
> > Hi Conor
> >
> > On Wed, 3 Apr 2024 at 17:14, Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Wed, Apr 03, 2024 at 09:03:47AM -0600, git@luigi311.com wrote:
> > > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > >
> > > > There are a number of variants of the imx258 modules that can not
> > > > be differentiated at runtime, so add compatible strings for the
> > > > PDAF variant.
> > > >
> > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > > Signed-off-by: Luis Garcia <git@luigi311.com>
> > > > ---
> > > > .../devicetree/bindings/media/i2c/sony,imx258.yaml | 9 +++++++--
> > > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > > index bee61a443b23..c978abc0cdb3 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > > @@ -13,11 +13,16 @@ description: |-
> > > > IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> > > > type stacked image sensor with a square pixel array of size 4208 x 3120. It
> > > > is programmable through I2C interface. Image data is sent through MIPI
> > > > - CSI-2.
> > > > + CSI-2. The sensor exists in two different models, a standard variant
> > > > + (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
> > > > + The camera module does not expose the model through registers, so the
> > > > + exact model needs to be specified.
> > > >
> > > > properties:
> > > > compatible:
> > > > - const: sony,imx258
> > > > + enum:
> > > > + - sony,imx258
> > > > + - sony,imx258-pdaf
> > >
> > > Does the pdaf variant support all of the features/is it register
> > > compatible with the regular variant? If it is, the regular variant
> > > should be a fallback compatible.
> >
> > It has the same register set, but certain registers have to be
> > programmed differently so that the image is corrected for the
> > partially shielded pixels used for phase detect auto focus (PDAF).
> > Either compatible will "work" on either variant of the module, but
> > you'll get weird image artifacts when using the wrong one.
>
> To paraphase, a fallback compatible is not suitable.
Whoops, I forgot this:
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add ft5426
From: Andreas Kemnade @ 2024-04-05 16:28 UTC (permalink / raw)
To: Andy Shevchenko
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
o.rempel, u.kleine-koenig, hdegoede, ye.xingchen, p.puschmann,
linux-input, devicetree, linux-kernel, caleb.connolly
In-Reply-To: <CAHp75VeZ9U_+1rJQjr4KvvzjYQGzfKtk+BK00vqvKcVn2-yP3g@mail.gmail.com>
On Fri, 5 Apr 2024 18:13:45 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Fri, Apr 5, 2024 at 1:20 AM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > As ft5426 seems to be compatible with this driver, add it.
> > Debug output during identification: Model "generic ft5x06 (79)", Rev. "
>
> ...
>
> > @@ -1484,6 +1484,7 @@ static const struct of_device_id edt_ft5x06_of_match[] = {
> > { .compatible = "edt,edt-ft5206", .data = &edt_ft5x06_data },
> > { .compatible = "edt,edt-ft5306", .data = &edt_ft5x06_data },
> > { .compatible = "edt,edt-ft5406", .data = &edt_ft5x06_data },
> > + { .compatible = "focaltech,ft5426", .data = &edt_ft5506_data },
>
> Why a different vendor prefix?
> In case you need to use this one, keep the list sorted, currently this
> splits the edt,* ones.
>
How do I know whether to use evervision or edt instead?
I sorted by the numbers. Looking at datasheets for other controllers I see
https://www.displayfuture.com/Display/datasheet/controller/FT5x06.pdf
it only mentions FocalTech Systems Co., Ltd.
So how the vendor prefixes are derived?
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
From: Mithil @ 2024-04-05 16:29 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
In-Reply-To: <ec7f77a7-2cf1-4ea6-b9c4-d4fe8a1673ab@linaro.org>
On Fri, Apr 5, 2024 at 9:27 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 05/04/2024 16:48, Mithil wrote:
> > So sorry about the 2nd patch being sent as a new mail, here is a new
> > patch with the changes as suggested
> >
> >> Please use subject prefixes matching the subsystem
> > Changed the patch name to match the folder history.
>
> Nothing improved. What the history tells you?
>
Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema"
Not really sure what else I should change.
> >
> >> Is it your full name?
> > Fixed it, my apologies.
> >
> >> Filename like compatible.
> > Fixed.
>
> Still not, compatible is omap4.
>
Sorry, seems like I was sending the old file again.
Will fix this.
> >
> >> Please open existing bindings and look how it is done there.
> > Changed it, is it fine now?
>
> You mean v2? I have no clue to what you are responding here, but no, v2
> did not improve much.
>
Again, could you guide me to what needs to be done?
Description for reg should be fine as this is how it is done in other
files as well.
Interrupts and hwmods use maxItems now.
Changed nodename to be generic in example as well.
Those were the suggested changes previously.
Best regards,
Mithil
^ permalink raw reply
* Re: [PATCH v4 2/2] phy: starfive: Add mipi dphy tx support
From: Vinod Koul @ 2024-04-05 16:31 UTC (permalink / raw)
To: Shengyang Chen
Cc: devicetree, linux-phy, kishon, robh+dt, krzysztof.kozlowski+dt,
conor+dt, p.zabel, minda.chen, changhuang.liang, rogerq,
geert+renesas, keith.zhao, linux-kernel
In-Reply-To: <20240301012406.92589-3-shengyang.chen@starfivetech.com>
On 01-03-24, 09:24, Shengyang Chen wrote:
> Add mipi dphy tx support for the StarFive JH7110 SoC.
> It is a module which is used to receive data from DSI
> driver and transfer data to DSI interface like mipi screen.
>
> Signed-off-by: Shengyang Chen <shengyang.chen@starfivetech.com>
> Reviewed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
> MAINTAINERS | 7 +
> drivers/phy/starfive/Kconfig | 10 +
> drivers/phy/starfive/Makefile | 1 +
> drivers/phy/starfive/phy-jh7110-dphy-tx.c | 702 ++++++++++++++++++++++
> 4 files changed, 720 insertions(+)
> create mode 100644 drivers/phy/starfive/phy-jh7110-dphy-tx.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ddbe6c82610f..13219c6504a9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20871,6 +20871,13 @@ S: Supported
> F: Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
> F: drivers/phy/starfive/phy-jh7110-dphy-rx.c
>
> +STARFIVE JH7110 DPHY TX DRIVER
> +M: Keith Zhao <keith.zhao@starfivetech.com>
> +M: Shengyang Chen <shengyang.chen@starfivetech.com>
> +S: Supported
> +F: Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-tx.yaml
> +F: drivers/phy/starfive/phy-jh7110-dphy-tx.c
> +
> STARFIVE JH7110 MMC/SD/SDIO DRIVER
> M: William Qiu <william.qiu@starfivetech.com>
> S: Supported
> diff --git a/drivers/phy/starfive/Kconfig b/drivers/phy/starfive/Kconfig
> index 9508e2143011..d0cdd7cb4a13 100644
> --- a/drivers/phy/starfive/Kconfig
> +++ b/drivers/phy/starfive/Kconfig
> @@ -15,6 +15,16 @@ config PHY_STARFIVE_JH7110_DPHY_RX
> system. If M is selected, the module will be called
> phy-jh7110-dphy-rx.ko.
>
> +config PHY_STARFIVE_JH7110_DPHY_TX
> + tristate "StarFive JH7110 D-PHY TX Support"
> + depends on HAS_IOMEM
> + select GENERIC_PHY
> + select GENERIC_PHY_MIPI_DPHY
> + help
> + Choose this option if you have a StarFive D-PHY TX in your
> + system. If M is selected, the module will be called
> + phy-jh7110-dphy-tx.ko.
> +
> config PHY_STARFIVE_JH7110_PCIE
> tristate "Starfive JH7110 PCIE 2.0/USB 3.0 PHY support"
> depends on HAS_IOMEM
> diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
> index b391018b7c47..eedc4a6fec15 100644
> --- a/drivers/phy/starfive/Makefile
> +++ b/drivers/phy/starfive/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_PHY_STARFIVE_JH7110_DPHY_RX) += phy-jh7110-dphy-rx.o
> +obj-$(CONFIG_PHY_STARFIVE_JH7110_DPHY_TX) += phy-jh7110-dphy-tx.o
> obj-$(CONFIG_PHY_STARFIVE_JH7110_PCIE) += phy-jh7110-pcie.o
> obj-$(CONFIG_PHY_STARFIVE_JH7110_USB) += phy-jh7110-usb.o
> diff --git a/drivers/phy/starfive/phy-jh7110-dphy-tx.c b/drivers/phy/starfive/phy-jh7110-dphy-tx.c
> new file mode 100644
> index 000000000000..df30cd424ba7
> --- /dev/null
> +++ b/drivers/phy/starfive/phy-jh7110-dphy-tx.c
> @@ -0,0 +1,702 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DPHY TX driver for the StarFive JH7110 SoC
> + *
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + * Author: Keith Zhao <keith.zhao@starfivetech.com>
> + * Author: Shengyang Chen <shengyang.chen@starfivetech.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/phy-mipi-dphy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#define STF_DPHY_APBIFSAIF_SYSCFG(x) (x)
> +
> +#define STF_DPHY_AON_POWER_READY_N_ACTIVE 0
> +#define STF_DPHY_AON_POWER_READY_N_SHIFT 0x0U
> +#define STF_DPHY_AON_POWER_READY_N_MASK BIT(0)
> +#define STF_DPHY_CFG_L0_SWAP_SEL_SHIFT 0xcU
> +#define STF_DPHY_CFG_L0_SWAP_SEL_MASK GENMASK(14, 12)
> +#define STF_DPHY_CFG_L1_SWAP_SEL_SHIFT 0xfU
> +#define STF_DPHY_CFG_L1_SWAP_SEL_MASK GENMASK(17, 15)
> +#define STF_DPHY_CFG_L2_SWAP_SEL_SHIFT 0x12U
> +#define STF_DPHY_CFG_L2_SWAP_SEL_MASK GENMASK(20, 18)
> +#define STF_DPHY_CFG_L3_SWAP_SEL_SHIFT 0x15U
> +#define STF_DPHY_CFG_L3_SWAP_SEL_MASK GENMASK(23, 21)
> +#define STF_DPHY_CFG_L4_SWAP_SEL_SHIFT 0x18U
> +#define STF_DPHY_CFG_L4_SWAP_SEL_MASK GENMASK(26, 24)
> +#define STF_DPHY_RGS_CDTX_PLL_UNLOCK_SHIFT 0x12U
> +#define STF_DPHY_RGS_CDTX_PLL_UNLOCK_MASK BIT(18)
> +#define STF_DPHY_RG_CDTX_L0N_HSTX_RES_SHIFT 0x13U
> +#define STF_DPHY_RG_CDTX_L0N_HSTX_RES_MASK GENMASK(23, 19)
> +#define STF_DPHY_RG_CDTX_L0P_HSTX_RES_SHIFT 0x18U
> +#define STF_DPHY_RG_CDTX_L0P_HSTX_RES_MASK GENMASK(28, 24)
> +
> +#define STF_DPHY_RG_CDTX_L1P_HSTX_RES_SHIFT 0x5U
> +#define STF_DPHY_RG_CDTX_L1P_HSTX_RES_MASK GENMASK(9, 5)
> +#define STF_DPHY_RG_CDTX_L2N_HSTX_RES_SHIFT 0xaU
> +#define STF_DPHY_RG_CDTX_L2N_HSTX_RES_MASK GENMASK(14, 10)
> +#define STF_DPHY_RG_CDTX_L2P_HSTX_RES_SHIFT 0xfU
> +#define STF_DPHY_RG_CDTX_L2P_HSTX_RES_MASK GENMASK(19, 15)
> +#define STF_DPHY_RG_CDTX_L3N_HSTX_RES_SHIFT 0x14U
> +#define STF_DPHY_RG_CDTX_L3N_HSTX_RES_MASK GENMASK(24, 20)
> +#define STF_DPHY_RG_CDTX_L3P_HSTX_RES_SHIFT 0x19U
> +#define STF_DPHY_RG_CDTX_L3P_HSTX_RES_MASK GENMASK(29, 25)
can you get rid of the shift defines. Please use FEILD_PREP and
FIELD_GET which uses the bitmasks you have defined about (you can drop
_MASK from above)
> +
> +#define STF_DPHY_RG_CDTX_L4N_HSTX_RES_SHIFT 0x0U
> +#define STF_DPHY_RG_CDTX_L4N_HSTX_RES_MASK GENMASK(4, 0)
> +#define STF_DPHY_RG_CDTX_L4P_HSTX_RES_SHIFT 0x5U
> +#define STF_DPHY_RG_CDTX_L4P_HSTX_RES_MASK GENMASK(9, 5)
> +#define STF_DPHY_RG_CDTX_PLL_FBK_FRA_SHIFT 0x0U
> +#define STF_DPHY_RG_CDTX_PLL_FBK_FRA_MASK GENMASK(23, 0)
> +
> +#define STF_DPHY_RG_CDTX_PLL_FBK_INT_SHIFT 0x0U
> +#define STF_DPHY_RG_CDTX_PLL_FBK_INT_MASK GENMASK(8, 0)
> +#define STF_DPHY_RG_CDTX_PLL_FM_EN_SHIFT 0x9U
> +#define STF_DPHY_RG_CDTX_PLL_FM_EN_MASK BIT(9)
> +#define STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_SHIFT 0xaU
> +#define STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_MASK BIT(10)
> +#define STF_DPHY_RG_CDTX_PLL_PRE_DIV_SHIFT 0xbU
> +#define STF_DPHY_RG_CDTX_PLL_PRE_DIV_MASK GENMASK(12, 11)
> +
> +#define STF_DPHY_RG_CDTX_PLL_SSC_EN_SHIFT 0x12U
> +#define STF_DPHY_RG_CDTX_PLL_SSC_EN_MASK 0x40000U
> +
> +#define STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_SHIFT 0x0U
> +#define STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_MASK GENMASK(7, 0)
> +#define STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_SHIFT 0x8U
> +#define STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_MASK GENMASK(15, 8)
> +#define STF_DPHY_RG_CLANE_HS_PRE_TIME_SHIFT 0x10U
> +#define STF_DPHY_RG_CLANE_HS_PRE_TIME_MASK GENMASK(23, 16)
> +#define STF_DPHY_RG_CLANE_HS_TRAIL_TIME_SHIFT 0x18U
> +#define STF_DPHY_RG_CLANE_HS_TRAIL_TIME_MASK GENMASK(31, 24)
> +
> +#define STF_DPHY_RG_CLANE_HS_ZERO_TIME_SHIFT 0x0U
> +#define STF_DPHY_RG_CLANE_HS_ZERO_TIME_MASK GENMASK(7, 0)
> +#define STF_DPHY_RG_DLANE_HS_PRE_TIME_SHIFT 0x8U
> +#define STF_DPHY_RG_DLANE_HS_PRE_TIME_MASK GENMASK(15, 8)
> +#define STF_DPHY_RG_DLANE_HS_TRAIL_TIME_SHIFT 0x10U
> +#define STF_DPHY_RG_DLANE_HS_TRAIL_TIME_MASK GENMASK(23, 16)
> +#define STF_DPHY_RG_DLANE_HS_ZERO_TIME_SHIFT 0x18U
> +#define STF_DPHY_RG_DLANE_HS_ZERO_TIME_MASK GENMASK(31, 24)
> +
> +#define STF_DPHY_RG_EXTD_CYCLE_SEL_SHIFT 0x0U
> +#define STF_DPHY_RG_EXTD_CYCLE_SEL_MASK GENMASK(2, 0)
> +#define STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_SHIFT 0x0U
> +#define STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_MASK GENMASK(31, 0)
> +
> +#define STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_SHIFT 0x1U
> +#define STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_MASK GENMASK(2, 1)
> +#define STF_DPHY_SCFG_PPI_C_READY_SEL_SHIFT 0x3U
> +#define STF_DPHY_SCFG_PPI_C_READY_SEL_MASK GENMASK(4, 3)
> +
> +#define STF_DPHY_REFCLK_IN_SEL_SHIFT 0x1aU
> +#define STF_DPHY_REFCLK_IN_SEL_MASK GENMASK(28, 26)
> +#define STF_DPHY_RESETB_SHIFT 0x1dU
> +#define STF_DPHY_RESETB_MASK BIT(29)
> +
> +#define STF_DPHY_REFCLK_12M 1
> +#define STF_DPHY_BITRATE_ALIGN 10000000
> +
> +#define STF_MAP_LANES_NUM 5
> +
> +#define STF_DPHY_LSHIFT_16(x) ((x) << 16)
> +#define STF_DPHY_LSHIFT_8(x) ((x) << 8)
these two should be dropped
> +
> +struct m31_dphy_config {
> + int ref_clk;
why is this signed?
> + unsigned long bitrate;
> + u32 pll_prev_div;
> + u32 pll_fbk_int;
> + u32 pll_fbk_fra;
> + u32 extd_cycle_sel;
> + u32 dlane_hs_pre_time;
> + u32 dlane_hs_zero_time;
> + u32 dlane_hs_trail_time;
> + u32 clane_hs_pre_time;
> + u32 clane_hs_zero_time;
> + u32 clane_hs_trail_time;
> + u32 clane_hs_clk_pre_time;
> + u32 clane_hs_clk_post_time;
> +};
> +
> +static const struct m31_dphy_config m31_dphy_configs[] = {
> + {12000000, 160000000, 0x0, 0x6a,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x3, 0xa, 0x17, 0x11, 0x5, 0x2b, 0xd, 0x7, 0x3d},
you can add this in a single line, that would be more readable...
> + {12000000, 170000000, 0x0, 0x71,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x3, 0xb, 0x18, 0x11, 0x5, 0x2e, 0xd, 0x7, 0x3d},
> + {12000000, 180000000, 0x0, 0x78,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x3, 0xb, 0x19, 0x12, 0x6, 0x30, 0xe, 0x7, 0x3e},
> + {12000000, 190000000, 0x0, 0x7e,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x3, 0xc, 0x1a, 0x12, 0x6, 0x33, 0xe, 0x7, 0x3e},
> + {12000000, 200000000, 0x0, 0x85,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x3, 0xc, 0x1b, 0x13, 0x7, 0x35, 0xf, 0x7, 0x3f},
> + {12000000, 320000000, 0x0, 0x6a,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0x8, 0x14, 0xf, 0x5, 0x2b, 0xd, 0x3, 0x23},
> + {12000000, 330000000, 0x0, 0x6e,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0x8, 0x15, 0xf, 0x5, 0x2d, 0xd, 0x3, 0x23},
> + {12000000, 340000000, 0x0, 0x71,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0x9, 0x15, 0xf, 0x5, 0x2e, 0xd, 0x3, 0x23},
> + {12000000, 350000000, 0x0, 0x74,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0x9, 0x15, 0x10, 0x6, 0x2f, 0xe, 0x3, 0x24},
> + {12000000, 360000000, 0x0, 0x78,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0x9, 0x16, 0x10, 0x6, 0x30, 0xe, 0x3, 0x24},
> + {12000000, 370000000, 0x0, 0x7b,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0x9, 0x17, 0x10, 0x6, 0x32, 0xe, 0x3, 0x24},
> + {12000000, 380000000, 0x0, 0x7e,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xa, 0x17, 0x10, 0x6, 0x33, 0xe, 0x3, 0x24},
> + {12000000, 390000000, 0x0, 0x82,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0, 0x2,
> + 0xa, 0x17, 0x11, 0x6, 0x35, 0xf, 0x3, 0x25},
> + {12000000, 400000000, 0x0, 0x85,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xa, 0x18, 0x11, 0x7, 0x35, 0xf, 0x3, 0x25},
> + {12000000, 410000000, 0x0, 0x88,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xa, 0x19, 0x11, 0x7, 0x37, 0xf, 0x3, 0x25},
> + {12000000, 420000000, 0x0, 0x8c,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0xa, 0x19, 0x12, 0x7, 0x38, 0x10, 0x3, 0x26},
> + {12000000, 430000000, 0x0, 0x8f,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xb, 0x19, 0x12, 0x7, 0x39, 0x10, 0x3, 0x26},
> + {12000000, 440000000, 0x0, 0x92,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xb, 0x1a, 0x12, 0x7, 0x3b, 0x10, 0x3, 0x26},
> + {12000000, 450000000, 0x0, 0x96,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0xb, 0x1b, 0x12, 0x8, 0x3c, 0x10, 0x3, 0x26},
> + {12000000, 460000000, 0x0, 0x99,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xb, 0x1b, 0x13, 0x8, 0x3d, 0x11, 0x3, 0x27},
> + {12000000, 470000000, 0x0, 0x9c,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xc, 0x1b, 0x13, 0x8, 0x3e, 0x11, 0x3, 0x27},
> + {12000000, 480000000, 0x0, 0xa0,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0xc, 0x1c, 0x13, 0x8, 0x40, 0x11, 0x3, 0x27},
> + {12000000, 490000000, 0x0, 0xa3,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xc, 0x1d, 0x14, 0x8, 0x42, 0x12, 0x3, 0x28},
> + {12000000, 500000000, 0x0, 0xa6,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xc, 0x1d, 0x14, 0x9, 0x42, 0x12, 0x3, 0x28},
> + {12000000, 510000000, 0x0, 0xaa,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0xc, 0x1e, 0x14, 0x9, 0x44, 0x12, 0x3, 0x28},
> + {12000000, 520000000, 0x0, 0xad,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xd, 0x1e, 0x15, 0x9, 0x45, 0x13, 0x3, 0x29},
> + {12000000, 530000000, 0x0, 0xb0,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xd, 0x1e, 0x15, 0x9, 0x47, 0x13, 0x3, 0x29},
> + {12000000, 540000000, 0x0, 0xb4,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0xd, 0x1f, 0x15, 0x9, 0x48, 0x13, 0x3, 0x29},
> + {12000000, 550000000, 0x0, 0xb7,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xd, 0x20, 0x16, 0x9, 0x4a, 0x14, 0x3, 0x2a},
> + {12000000, 560000000, 0x0, 0xba,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xe, 0x20, 0x16, 0xa, 0x4a, 0x14, 0x3, 0x2a},
> + {12000000, 570000000, 0x0, 0xbe,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0xe, 0x20, 0x16, 0xa, 0x4c, 0x14, 0x3, 0x2a},
> + {12000000, 580000000, 0x0, 0xc1,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xe, 0x21, 0x16, 0xa, 0x4d, 0x14, 0x3, 0x2a},
> + {12000000, 590000000, 0x0, 0xc4,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xe, 0x22, 0x17, 0xa, 0x4f, 0x15, 0x3, 0x2b},
> + {12000000, 600000000, 0x0, 0xc8,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x2, 0xe, 0x23, 0x17, 0xa, 0x50, 0x15, 0x3, 0x2b},
> + {12000000, 610000000, 0x0, 0xcb,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x2, 0xf, 0x22, 0x17, 0xb, 0x50, 0x15, 0x3, 0x2b},
> + {12000000, 620000000, 0x0, 0xce,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x2, 0xf, 0x23, 0x18, 0xb, 0x52, 0x16, 0x3, 0x2c},
> + {12000000, 630000000, 0x0, 0x69,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x7, 0x12, 0xd, 0x5, 0x2a, 0xc, 0x1, 0x15},
> + {12000000, 640000000, 0x0, 0x6a,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0x7, 0x13, 0xe, 0x5, 0x2b, 0xd, 0x1, 0x16},
> + {12000000, 650000000, 0x0, 0x6c,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0x7, 0x13, 0xe, 0x5, 0x2c, 0xd, 0x1, 0x16},
> + {12000000, 660000000, 0x0, 0x6e,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x7, 0x13, 0xe, 0x5, 0x2d, 0xd, 0x1, 0x16},
> + {12000000, 670000000, 0x0, 0x6f,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0x8, 0x13, 0xe, 0x5, 0x2d, 0xd, 0x1, 0x16},
> + {12000000, 680000000, 0x0, 0x71,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0x8, 0x13, 0xe, 0x5, 0x2e, 0xd, 0x1, 0x16},
> + {12000000, 690000000, 0x0, 0x73,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x8, 0x14, 0xe, 0x6, 0x2e, 0xd, 0x1, 0x16},
> + {12000000, 700000000, 0x0, 0x74,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0x8, 0x14, 0xf, 0x6, 0x2f, 0xe, 0x1, 0x16},
> + {12000000, 710000000, 0x0, 0x76,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0x8, 0x14, 0xf, 0x6, 0x2f, 0xe, 0x1, 0x17},
> + {12000000, 720000000, 0x0, 0x78,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x8, 0x15, 0xf, 0x6, 0x30, 0xe, 0x1, 0x17},
> + {12000000, 730000000, 0x0, 0x79,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0x8, 0x15, 0xf, 0x6, 0x31, 0xe, 0x1, 0x17},
> + {12000000, 740000000, 0x0, 0x7b,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0x8, 0x15, 0xf, 0x6, 0x32, 0xe, 0x1, 0x17},
> + {12000000, 750000000, 0x0, 0x7d,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x8, 0x16, 0xf, 0x6, 0x32, 0xe, 0x1, 0x17},
> + {12000000, 760000000, 0x0, 0x7e,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0x9, 0x15, 0xf, 0x6, 0x33, 0xe, 0x1, 0x17},
> + {12000000, 770000000, 0x0, 0x80,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0x9, 0x15, 0x10, 0x6, 0x34, 0xf, 0x1, 0x18},
> + {12000000, 780000000, 0x0, 0x82,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x9, 0x16, 0x10, 0x6, 0x35, 0xf, 0x1, 0x18,},
> + {12000000, 790000000, 0x0, 0x83,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0x9, 0x16, 0x10, 0x7, 0x34, 0xf, 0x1, 0x18},
> + {12000000, 800000000, 0x0, 0x85,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0x9, 0x17, 0x10, 0x7, 0x35, 0xf, 0x1, 0x18},
> + {12000000, 810000000, 0x0, 0x87,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x9, 0x17, 0x10, 0x7, 0x36, 0xf, 0x1, 0x18},
> + {12000000, 820000000, 0x0, 0x88,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0x9, 0x17, 0x10, 0x7, 0x37, 0xf, 0x1, 0x18},
> + {12000000, 830000000, 0x0, 0x8a,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0x9, 0x18, 0x10, 0x7, 0x37, 0xf, 0x1, 0x18},
> + {12000000, 840000000, 0x0, 0x8c,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0x9, 0x18, 0x11, 0x7, 0x38, 0x10, 0x1, 0x19},
> + {12000000, 850000000, 0x0, 0x8d,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0xa, 0x17, 0x11, 0x7, 0x39, 0x10, 0x1, 0x19},
> + {12000000, 860000000, 0x0, 0x8f,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0xa, 0x18, 0x11, 0x7, 0x39, 0x10, 0x1, 0x19},
> + {12000000, 870000000, 0x0, 0x91,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0xa, 0x18, 0x11, 0x7, 0x3a, 0x10, 0x1, 0x19},
> + {12000000, 880000000, 0x0, 0x92,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0xa, 0x18, 0x11, 0x7, 0x3b, 0x10, 0x1, 0x19},
> + {12000000, 890000000, 0x0, 0x94,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0xa, 0x19, 0x11, 0x7, 0x3c, 0x10, 0x1, 0x19},
> + {12000000, 900000000, 0x0, 0x96,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0xa, 0x19, 0x12, 0x8, 0x3c, 0x10, 0x1, 0x19},
> + {12000000, 910000000, 0x0, 0x97,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0xa, 0x19, 0x12, 0x8, 0x3c, 0x11, 0x1, 0x1a},
> + {12000000, 920000000, 0x0, 0x99,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0xa, 0x1a, 0x12, 0x8, 0x3d, 0x11, 0x1, 0x1a},
> + {12000000, 930000000, 0x0, 0x9b,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0xa, 0x1a, 0x12, 0x8, 0x3e, 0x11, 0x1, 0x1a},
> + {12000000, 940000000, 0x0, 0x9c,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0xb, 0x1a, 0x12, 0x8, 0x3e, 0x11, 0x1, 0x1a},
> + {12000000, 950000000, 0x0, 0x9e,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0xb, 0x1a, 0x12, 0x8, 0x3f, 0x11, 0x1, 0x1a},
> + {12000000, 960000000, 0x0, 0xa0,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0xb, 0x1a, 0x12, 0x8, 0x40, 0x11, 0x1, 0x1a},
> + {12000000, 970000000, 0x0, 0xa1,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0xb, 0x1b, 0x13, 0x8, 0x41, 0x12, 0x1, 0x1b},
> + {12000000, 980000000, 0x0, 0xa3,
> + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> + 0x1, 0xb, 0x1b, 0x13, 0x8, 0x42, 0x12, 0x1, 0x1b},
> + {12000000, 990000000, 0x0, 0xa5,
> + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> + 0x1, 0xb, 0x1b, 0x13, 0x8, 0x42, 0x12, 0x1, 0x1b},
> + {12000000, 1000000000, 0x0, 0xa6,
> + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> + 0x1, 0xb, 0x1c, 0x13, 0x9, 0x42, 0x12, 0x1, 0x1b},
> +};
> +
> +struct stf_dphy_info {
> + /**
> + * @maps:
> + *
> + * Physical lanes and logic lanes mapping table.
> + *
> + * The default order is:
> + * [data lane 0, data lane 1, data lane 2, date lane 3, clk lane]
> + */
> + u8 maps[STF_MAP_LANES_NUM];
> +};
> +
> +struct stf_dphy {
> + struct device *dev;
> + void __iomem *topsys;
> + struct clk *txesc_clk;
> + struct reset_control *sys_rst;
> +
> + struct phy_configure_opts_mipi_dphy config;
> +
> + struct phy *phy;
> + const struct stf_dphy_info *info;
> +};
> +
> +static inline u32 stf_dphy_get_reg(void __iomem *io_addr, u32 addr, u32 shift, u32 mask)
> +{
> + u32 tmp;
> +
> + tmp = readl(io_addr);
> + tmp = (tmp & mask) >> shift;
> + return tmp;
> +}
> +
> +static inline void stf_dphy_set_reg(void __iomem *io_addr, u32 addr, u32 data, u32 shift, u32 mask)
> +{
> + u32 tmp;
> +
> + tmp = readl(io_addr + addr);
> + tmp &= ~mask;
> + tmp |= (data << shift) & mask;
> + writel(tmp, (io_addr + addr));
> +}
> +
> +static int is_pll_locked(struct stf_dphy *dphy)
> +{
> + int tmp = stf_dphy_get_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(8),
> + STF_DPHY_RGS_CDTX_PLL_UNLOCK_SHIFT,
> + STF_DPHY_RGS_CDTX_PLL_UNLOCK_MASK);
> + return !tmp;
why inverted value?
> +}
> +
> +static void stf_dphy_hw_reset(struct stf_dphy *dphy, int assert)
> +{
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(100),
> + !assert, STF_DPHY_RESETB_SHIFT, STF_DPHY_RESETB_MASK);
> +
> + if (!assert) {
> + while ((!is_pll_locked(dphy)))
> + ;
This is infinite, that is not acceptable. Please have a decent timeout
for this...
> + dev_err(dphy->dev, "MIPI dphy-tx # PLL Locked\n");
> + }
> +}
> +
> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> + const struct stf_dphy_info *info = dphy->info;
> + u32 bitrate = opts->mipi_dphy.hs_clk_rate;
> + const struct m31_dphy_config *p;
> + unsigned long alignment;
> + int i;
> +
> + bitrate = opts->mipi_dphy.hs_clk_rate;
> +
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(8), 0x10,
> + STF_DPHY_RG_CDTX_L0N_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L0N_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> + STF_DPHY_RG_CDTX_L0N_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L0N_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> + STF_DPHY_RG_CDTX_L2N_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L2N_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> + STF_DPHY_RG_CDTX_L3N_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L3N_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(16), 0x10,
> + STF_DPHY_RG_CDTX_L4N_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L4N_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(8), 0x10,
> + STF_DPHY_RG_CDTX_L0P_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L0P_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> + STF_DPHY_RG_CDTX_L1P_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L1P_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> + STF_DPHY_RG_CDTX_L2P_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L2P_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> + STF_DPHY_RG_CDTX_L3P_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L3P_HSTX_RES_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(16), 0x10,
> + STF_DPHY_RG_CDTX_L4P_HSTX_RES_SHIFT, STF_DPHY_RG_CDTX_L4P_HSTX_RES_MASK);
> +
> + alignment = STF_DPHY_BITRATE_ALIGN;
> + if (bitrate % alignment)
> + bitrate += alignment - (bitrate % alignment);
> +
> + p = m31_dphy_configs;
> + for (i = 0; i < ARRAY_SIZE(m31_dphy_configs); i++, p++) {
> + if (p->bitrate == bitrate) {
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(100),
> + STF_DPHY_REFCLK_12M, STF_DPHY_REFCLK_IN_SEL_SHIFT,
> + STF_DPHY_REFCLK_IN_SEL_MASK);
> +
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(0),
> + STF_DPHY_AON_POWER_READY_N_ACTIVE,
> + STF_DPHY_AON_POWER_READY_N_SHIFT,
> + STF_DPHY_AON_POWER_READY_N_MASK);
> +
> + /*Lane setting*/
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[0],
> + STF_DPHY_CFG_L0_SWAP_SEL_SHIFT,
> + STF_DPHY_CFG_L0_SWAP_SEL_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[1],
> + STF_DPHY_CFG_L1_SWAP_SEL_SHIFT,
> + STF_DPHY_CFG_L1_SWAP_SEL_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[2],
> + STF_DPHY_CFG_L2_SWAP_SEL_SHIFT,
> + STF_DPHY_CFG_L2_SWAP_SEL_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[3],
> + STF_DPHY_CFG_L3_SWAP_SEL_SHIFT,
> + STF_DPHY_CFG_L3_SWAP_SEL_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[4],
> + STF_DPHY_CFG_L4_SWAP_SEL_SHIFT,
> + STF_DPHY_CFG_L4_SWAP_SEL_MASK);
> + /*PLL setting*/
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(28), 0x0,
> + STF_DPHY_RG_CDTX_PLL_SSC_EN_SHIFT,
> + STF_DPHY_RG_CDTX_PLL_SSC_EN_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(24), 0x1,
> + STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_SHIFT,
> + STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(24), 0x1,
> + STF_DPHY_RG_CDTX_PLL_FM_EN_SHIFT,
> + STF_DPHY_RG_CDTX_PLL_FM_EN_MASK);
> +
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(24),
> + p->pll_prev_div, STF_DPHY_RG_CDTX_PLL_PRE_DIV_SHIFT,
> + STF_DPHY_RG_CDTX_PLL_PRE_DIV_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(24),
> + p->pll_fbk_int, STF_DPHY_RG_CDTX_PLL_FBK_INT_SHIFT,
> + STF_DPHY_RG_CDTX_PLL_FBK_INT_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(20),
> + p->pll_fbk_fra, STF_DPHY_RG_CDTX_PLL_FBK_FRA_SHIFT,
> + STF_DPHY_RG_CDTX_PLL_FBK_FRA_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(40),
> + p->extd_cycle_sel, STF_DPHY_RG_EXTD_CYCLE_SEL_SHIFT,
> + STF_DPHY_RG_EXTD_CYCLE_SEL_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(36),
> + p->dlane_hs_pre_time,
> + STF_DPHY_RG_DLANE_HS_PRE_TIME_SHIFT,
> + STF_DPHY_RG_DLANE_HS_PRE_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(36),
> + p->dlane_hs_pre_time,
> + STF_DPHY_RG_DLANE_HS_PRE_TIME_SHIFT,
> + STF_DPHY_RG_DLANE_HS_PRE_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(36),
> + p->dlane_hs_zero_time,
> + STF_DPHY_RG_DLANE_HS_ZERO_TIME_SHIFT,
> + STF_DPHY_RG_DLANE_HS_ZERO_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(36),
> + p->dlane_hs_trail_time,
> + STF_DPHY_RG_DLANE_HS_TRAIL_TIME_SHIFT,
> + STF_DPHY_RG_DLANE_HS_TRAIL_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(32),
> + p->clane_hs_pre_time,
> + STF_DPHY_RG_CLANE_HS_PRE_TIME_SHIFT,
> + STF_DPHY_RG_CLANE_HS_PRE_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(36),
> + p->clane_hs_zero_time,
> + STF_DPHY_RG_CLANE_HS_ZERO_TIME_SHIFT,
> + STF_DPHY_RG_CLANE_HS_ZERO_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(32),
> + p->clane_hs_trail_time,
> + STF_DPHY_RG_CLANE_HS_TRAIL_TIME_SHIFT,
> + STF_DPHY_RG_CLANE_HS_TRAIL_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(32),
> + p->clane_hs_clk_pre_time,
> + STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_SHIFT,
> + STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(32),
> + p->clane_hs_clk_post_time,
> + STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_SHIFT,
> + STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_MASK);
> +
> + break;
> + }
> + }
> +
> + return 0;
this is the only return. It will always return success, consider making
this return void
> +}
> +
> +static int stf_dphy_init(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> + int ret;
> +
> + stf_dphy_hw_reset(dphy, 0);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(48), 0,
> + STF_DPHY_SCFG_PPI_C_READY_SEL_SHIFT, STF_DPHY_SCFG_PPI_C_READY_SEL_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(48), 0,
> + STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_SHIFT,
> + STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_MASK);
> + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(44), 0x30,
> + STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_SHIFT,
> + STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_MASK);
> +
> + ret = clk_prepare_enable(dphy->txesc_clk);
> + if (ret) {
> + dev_err(dphy->dev, "Failed to prepare/enable txesc_clk\n");
> + return ret;
> + }
> +
> + ret = reset_control_deassert(dphy->sys_rst);
> + if (ret) {
> + dev_err(dphy->dev, "Failed to deassert sys_rst\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int stf_dphy_exit(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> + int ret;
> +
> + ret = reset_control_assert(dphy->sys_rst);
> + if (ret) {
> + dev_err(dphy->dev, "Failed to assert sys_rst\n");
> + return ret;
> + }
> +
> + clk_disable_unprepare(dphy->txesc_clk);
> +
> + stf_dphy_hw_reset(dphy, 1);
> +
> + return 0;
> +}
> +
> +static int stf_dphy_power_on(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> +
> + return pm_runtime_resume_and_get(dphy->dev);
> +}
> +
> +static int stf_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
> + union phy_configure_opts *opts)
> +{
> + if (mode != PHY_MODE_MIPI_DPHY)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int stf_dphy_power_off(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> +
> + return pm_runtime_put_sync(dphy->dev);
> +}
> +
> +static const struct phy_ops stf_dphy_ops = {
> + .power_on = stf_dphy_power_on,
> + .power_off = stf_dphy_power_off,
> + .init = stf_dphy_init,
> + .exit = stf_dphy_exit,
> + .configure = stf_dphy_configure,
> + .validate = stf_dphy_validate,
> + .owner = THIS_MODULE,
> +};
> +
> +static int stf_dphy_probe(struct platform_device *pdev)
> +{
> + struct phy_provider *phy_provider;
> + struct stf_dphy *dphy;
> +
> + dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
> + if (!dphy)
> + return -ENOMEM;
> +
> + dphy->info = of_device_get_match_data(&pdev->dev);
> +
> + dphy->dev = &pdev->dev;
> + dev_set_drvdata(&pdev->dev, dphy);
> +
> + dphy->topsys = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(dphy->topsys))
> + return PTR_ERR(dphy->topsys);
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + dphy->txesc_clk = devm_clk_get(&pdev->dev, "txesc");
> + if (IS_ERR(dphy->txesc_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dphy->txesc_clk),
> + "Failed to get txesc clock\n");
> +
> + dphy->sys_rst = devm_reset_control_get_exclusive(&pdev->dev, "sys");
> + if (IS_ERR(dphy->sys_rst))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dphy->sys_rst),
> + "Failed to get sys reset\n");
> +
> + dphy->phy = devm_phy_create(&pdev->dev, NULL, &stf_dphy_ops);
> + if (IS_ERR(dphy->phy))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dphy->phy),
> + "Failed to create phy\n");
> +
> + phy_set_drvdata(dphy->phy, dphy);
> +
> + phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
> + if (IS_ERR(phy_provider))
> + return dev_err_probe(&pdev->dev, PTR_ERR(phy_provider),
> + "Failed to register phy\n");
> +
> + return 0;
> +}
> +
> +static const struct stf_dphy_info starfive_dphy_info = {
> + .maps = {0, 1, 2, 3, 4},
> +};
> +
> +static const struct of_device_id stf_dphy_dt_ids[] = {
> + {
> + .compatible = "starfive,jh7110-dphy-tx",
> + .data = &starfive_dphy_info,
> + },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, stf_dphy_dt_ids);
> +
> +static struct platform_driver stf_dphy_driver = {
> + .driver = {
> + .name = "starfive-dphy-tx",
> + .of_match_table = stf_dphy_dt_ids,
> + },
> + .probe = stf_dphy_probe,
> +};
> +module_platform_driver(stf_dphy_driver);
> +
> +MODULE_AUTHOR("Keith Zhao <keith.zhao@starfivetech.com>");
> +MODULE_AUTHOR("Shengyang Chen <shengyang.chen@starfivetech.com>");
> +MODULE_DESCRIPTION("StarFive JH7110 DPHY TX driver");
> +MODULE_LICENSE("GPL");
> --
> 2.17.1
>
>
> --
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
From: Marco Felsch @ 2024-04-05 16:44 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi, devicetree, Peng Fan,
linux-kernel, linux-arm-kernel, imx
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-4-9fc9186856c2@nxp.com>
Hi Peng,
On 24-04-05, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> The i.MX MISC protocol is for misc settings, such as gpio expander
> wakeup.
Can you elaborate a bit more please?
Regards,
Marco
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/Kconfig | 10 ++
> drivers/firmware/arm_scmi/Makefile | 1 +
> drivers/firmware/arm_scmi/imx-sm-misc.c | 305 ++++++++++++++++++++++++++++++++
> include/linux/scmi_imx_protocol.h | 17 ++
> 4 files changed, 333 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
> index 56d11c9d9f47..bfeae92f6420 100644
> --- a/drivers/firmware/arm_scmi/Kconfig
> +++ b/drivers/firmware/arm_scmi/Kconfig
> @@ -191,3 +191,13 @@ config IMX_SCMI_BBM_EXT
> and BUTTON.
>
> This driver can also be built as a module.
> +
> +config IMX_SCMI_MISC_EXT
> + tristate "i.MX SCMI MISC EXTENSION"
> + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
> + default y if ARCH_MXC
> + help
> + This enables i.MX System MISC control logic such as gpio expander
> + wakeup
> +
> + This driver can also be built as a module.
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index 327687acf857..a23fde721222 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -12,6 +12,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
> scmi-protocols-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
> +scmi-protocols-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o
> scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> diff --git a/drivers/firmware/arm_scmi/imx-sm-misc.c b/drivers/firmware/arm_scmi/imx-sm-misc.c
> new file mode 100644
> index 000000000000..1b0ec2281518
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/imx-sm-misc.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System control and Management Interface (SCMI) NXP MISC Protocol
> + *
> + * Copyright 2024 NXP
> + */
> +
> +#define pr_fmt(fmt) "SCMI Notifications MISC - " fmt
> +
> +#include <linux/bits.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_imx_protocol.h>
> +
> +#include "protocols.h"
> +#include "notify.h"
> +
> +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
> +
> +enum scmi_imx_misc_protocol_cmd {
> + SCMI_IMX_MISC_CTRL_SET = 0x3,
> + SCMI_IMX_MISC_CTRL_GET = 0x4,
> + SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
> +};
> +
> +struct scmi_imx_misc_info {
> + u32 version;
> + u32 nr_dev_ctrl;
> + u32 nr_brd_ctrl;
> + u32 nr_reason;
> +};
> +
> +struct scmi_msg_imx_misc_protocol_attributes {
> + __le32 attributes;
> +};
> +
> +#define GET_BRD_CTRLS_NR(x) le32_get_bits((x), GENMASK(31, 24))
> +#define GET_REASONS_NR(x) le32_get_bits((x), GENMASK(23, 16))
> +#define GET_DEV_CTRLS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> +#define BRD_CTRL_START_ID BIT(15)
> +
> +struct scmi_imx_misc_ctrl_set_in {
> + __le32 id;
> + __le32 num;
> + __le32 value[MISC_MAX_VAL];
> +};
> +
> +struct scmi_imx_misc_ctrl_notify_in {
> + __le32 ctrl_id;
> + __le32 flags;
> +};
> +
> +struct scmi_imx_misc_ctrl_notify_payld {
> + __le32 ctrl_id;
> + __le32 flags;
> +};
> +
> +struct scmi_imx_misc_ctrl_get_out {
> + __le32 num;
> + __le32 *val;
> +};
> +
> +static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle *ph,
> + struct scmi_imx_misc_info *mi)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_imx_misc_protocol_attributes *attr;
> +
> + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0,
> + sizeof(*attr), &t);
> + if (ret)
> + return ret;
> +
> + attr = t->rx.buf;
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + mi->nr_dev_ctrl = GET_DEV_CTRLS_NR(attr->attributes);
> + mi->nr_brd_ctrl = GET_BRD_CTRLS_NR(attr->attributes);
> + mi->nr_reason = GET_REASONS_NR(attr->attributes);
> + dev_info(ph->dev, "i.MX MISC NUM DEV CTRL: %d, NUM BRD CTRL: %d,NUM Reason: %d\n",
> + mi->nr_dev_ctrl, mi->nr_brd_ctrl, mi->nr_reason);
> + }
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_validate_id(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id)
> +{
> + struct scmi_imx_misc_info *mi = ph->get_priv(ph);
> +
> + if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi->nr_dev_ctrl))
> + return -EINVAL;
> + if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int scmi_imx_misc_ctrl_notify(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id, u32 flags)
> +{
> + struct scmi_imx_misc_ctrl_notify_in *in;
> + struct scmi_xfer *t;
> + int ret;
> +
> + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_NOTIFY,
> + sizeof(*in), 0, &t);
> + if (ret)
> + return ret;
> +
> + in = t->tx.buf;
> + in->ctrl_id = cpu_to_le32(ctrl_id);
> + in->flags = cpu_to_le32(flags);
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int
> +scmi_imx_misc_ctrl_set_notify_enabled(const struct scmi_protocol_handle *ph,
> + u8 evt_id, u32 src_id, bool enable)
> +{
> + int ret;
> +
> + ret = scmi_imx_misc_ctrl_notify(ph, src_id, enable ? evt_id : 0);
> + if (ret)
> + dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] - ret:%d\n",
> + evt_id, src_id, ret);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_get_num_sources(const struct scmi_protocol_handle *ph)
> +{
> + return GENMASK(15, 0);
> +}
> +
> +static void *
> +scmi_imx_misc_ctrl_fill_custom_report(const struct scmi_protocol_handle *ph,
> + u8 evt_id, ktime_t timestamp,
> + const void *payld, size_t payld_sz,
> + void *report, u32 *src_id)
> +{
> + const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
> + struct scmi_imx_misc_ctrl_notify_report *r = report;
> +
> + if (sizeof(*p) != payld_sz)
> + return NULL;
> +
> + r->timestamp = timestamp;
> + r->ctrl_id = p->ctrl_id;
> + r->flags = p->flags;
> + *src_id = r->ctrl_id;
> + dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
> + r->ctrl_id, r->flags);
> +
> + return r;
> +}
> +
> +static const struct scmi_event_ops scmi_imx_misc_event_ops = {
> + .get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
> + .set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
> + .fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
> +};
> +
> +static const struct scmi_event scmi_imx_misc_events[] = {
> + {
> + .id = SCMI_EVENT_IMX_MISC_CONTROL_DISABLED,
> + .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
> + .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
> + },
> + {
> + .id = SCMI_EVENT_IMX_MISC_CONTROL_FALLING_EDGE,
> + .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
> + .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
> + },
> + {
> + .id = SCMI_EVENT_IMX_MISC_CONTROL_RISING_EDGE,
> + .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
> + .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
> + }
> +};
> +
> +static struct scmi_protocol_events scmi_imx_misc_protocol_events = {
> + .queue_sz = SCMI_PROTO_QUEUE_SZ,
> + .ops = &scmi_imx_misc_event_ops,
> + .evts = scmi_imx_misc_events,
> + .num_events = ARRAY_SIZE(scmi_imx_misc_events),
> +};
> +
> +static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
> +{
> + struct scmi_imx_misc_info *minfo;
> + u32 version;
> + int ret;
> +
> + ret = ph->xops->version_get(ph, &version);
> + if (ret)
> + return ret;
> +
> + dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
> + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
> +
> + minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
> + if (!minfo)
> + return -ENOMEM;
> +
> + ret = scmi_imx_misc_attributes_get(ph, minfo);
> + if (ret)
> + return ret;
> +
> + return ph->set_priv(ph, minfo, version);
> +}
> +
> +static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id, u32 *num, u32 *val)
> +{
> + struct scmi_imx_misc_ctrl_get_out *out;
> + struct scmi_xfer *t;
> + int ret, i;
> +
> + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET, sizeof(u32),
> + 0, &t);
> + if (ret)
> + return ret;
> +
> + put_unaligned_le32(ctrl_id, t->tx.buf);
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + out = t->rx.buf;
> + *num = le32_to_cpu(out->num);
> + for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
> + val[i] = le32_to_cpu(out->val[i]);
> + }
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle *ph,
> + u32 ctrl_id, u32 num, u32 *val)
> +{
> + struct scmi_imx_misc_ctrl_set_in *in;
> + struct scmi_xfer *t;
> + int ret, i;
> +
> + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> + if (ret)
> + return ret;
> +
> + if (num > MISC_MAX_VAL)
> + return -EINVAL;
> +
> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET, sizeof(*in),
> + 0, &t);
> + if (ret)
> + return ret;
> +
> + in = t->tx.buf;
> + in->id = cpu_to_le32(ctrl_id);
> + in->num = cpu_to_le32(num);
> + for (i = 0; i < num; i++)
> + in->value[i] = cpu_to_le32(val[i]);
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
> + .misc_ctrl_set = scmi_imx_misc_ctrl_set,
> + .misc_ctrl_get = scmi_imx_misc_ctrl_get,
> +};
> +
> +static const struct scmi_protocol scmi_imx_misc = {
> + .id = SCMI_PROTOCOL_IMX_MISC,
> + .owner = THIS_MODULE,
> + .instance_init = &scmi_imx_misc_protocol_init,
> + .ops = &scmi_imx_misc_proto_ops,
> + .events = &scmi_imx_misc_protocol_events,
> + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
> +};
> +module_scmi_protocol(scmi_imx_misc);
> diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
> index 90ce011a4429..a69bd4a20f0f 100644
> --- a/include/linux/scmi_imx_protocol.h
> +++ b/include/linux/scmi_imx_protocol.h
> @@ -13,8 +13,14 @@
> #include <linux/notifier.h>
> #include <linux/types.h>
>
> +#define SCMI_PAYLOAD_LEN 100
> +
> +#define SCMI_ARRAY(X, Y) ((SCMI_PAYLOAD_LEN - (X)) / sizeof(Y))
> +#define MISC_MAX_VAL SCMI_ARRAY(8, uint32_t)
> +
> enum scmi_nxp_protocol {
> SCMI_PROTOCOL_IMX_BBM = 0x81,
> + SCMI_PROTOCOL_IMX_MISC = 0x84,
> };
>
> struct scmi_imx_bbm_proto_ops {
> @@ -42,4 +48,15 @@ struct scmi_imx_bbm_notif_report {
> unsigned int rtc_id;
> unsigned int rtc_evt;
> };
> +
> +struct scmi_imx_misc_ctrl_notify_report {
> + ktime_t timestamp;
> + unsigned int ctrl_id;
> + unsigned int flags;
> +};
> +
> +struct scmi_imx_misc_proto_ops {
> + int (*misc_ctrl_set)(const struct scmi_protocol_handle *ph, u32 id, u32 num, u32 *val);
> + int (*misc_ctrl_get)(const struct scmi_protocol_handle *ph, u32 id, u32 *num, u32 *val);
> +};
> #endif
>
> --
> 2.37.1
>
>
>
^ permalink raw reply
* Re: [PATCH v1 1/4] arm64: dts: freescale: imx8mp-verdin: replace sleep-moci hog with regulator
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-2-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:17PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> The Verdin family has a signal called sleep-moci which can be used to
> turn off peripherals on the carrier board when the SoM goes into
> suspend. So far we have hogged this signal, which means the peripherals
> are always on and it is not possible to add peripherals that depend on
> the sleep-moci to be on. With this change, we replace the hog with a
> regulator so that peripherals can add their own regulators that use the
> same gpio. Carrier boards that allow peripherals to be powered off in
> suspend can disable this regulator and implement their own regulator to
> control the sleep-moci.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
* Re: [PATCH v1 2/4] arm64: dts: freescale: imx8mp-verdin-dahlia: support sleep-moci
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-3-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:18PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Previously, we had the sleep-moci pin set to always on. However, the
> Dahlia carrier board supports disabling the sleep-moci when the system
> is suspended to power down peripherals that support it. This reduces
> overall power consumption. This commit adds support for this feature by
> disabling the reg_force_sleep_moci regulator and adding two new
> regulators for the USB hub and PCIe that can be turned off when the
> system is suspended.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
* Re: [PATCH v1 3/4] arm64: dts: freescale: imx8mm-verdin: replace sleep-moci hog with regulator
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-4-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:19PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> The Verdin family has a signal called sleep-moci which can be used to
> turn off peripherals on the carrier board when the SoM goes into
> suspend. So far we have hogged this signal, which means the peripherals
> are always on and it is not possible to add peripherals that depend on
> the sleep-moci to be on. With this change, we replace the hog with a
> regulator so that peripherals can add their own regulators that use the
> same gpio. Carrier boards that allow peripherals to be powered off in
> suspend can disable this regulator and implement their own regulator to
> control the sleep-moci.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
* Re: [PATCH v1 4/4] arm64: dts: freescale: imx8mm-verdin-dahlia: support sleep-moci
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-5-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:20PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Previously, we had the sleep-moci pin set to always on. However, the
> Dahlia carrier board supports disabling the sleep-moci when the system
> is suspended to power down peripherals that support it. This reduces
> overall power consumption. This commit adds support for this feature by
> disabling the reg_force_sleep_moci regulator and adding two new
> regulators for the USB hub and PCIe that can be turned off when the
> system is suspended.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
From: Vinod Koul @ 2024-04-05 16:52 UTC (permalink / raw)
To: forbidden405
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
In-Reply-To: <20240305-inno-phy-v5-5-dc1cb130ea08@outlook.com>
On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
That is quite an email id!
>
> Direct MMIO resgiter access is used by Hi3798MV200. For other models,
> of_iomap() returns NULL due to insufficient length. So they are
so how is that fixed... Pls describe the change...
> unaffected.
>
> Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
> switch to reset_control_array_*() APIs for that.
That probably should be a different patch
>
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---
> drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 66 ++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> index b7e740eb4752..df154cd99ed8 100644
> --- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> +++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> @@ -10,6 +10,7 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/reset.h>
> @@ -24,6 +25,7 @@
>
> #define PHY_TYPE_0 0
> #define PHY_TYPE_1 1
> +#define PHY_TYPE_MMIO 2
>
> #define PHY_TEST_DATA GENMASK(7, 0)
> #define PHY_TEST_ADDR_OFFSET 8
> @@ -43,6 +45,7 @@
> #define PHY_CLK_ENABLE BIT(2)
>
> struct hisi_inno_phy_port {
> + void __iomem *base;
> struct reset_control *utmi_rst;
> struct hisi_inno_phy_priv *priv;
> };
> @@ -50,7 +53,7 @@ struct hisi_inno_phy_port {
> struct hisi_inno_phy_priv {
> void __iomem *mmio;
> struct clk *ref_clk;
> - struct reset_control *por_rst;
> + struct reset_control *rsts;
> unsigned int type;
> struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
> };
> @@ -62,26 +65,31 @@ static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
> u32 val;
> u32 value;
>
> - if (priv->type == PHY_TYPE_0)
> - val = (data & PHY_TEST_DATA) |
> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
> - ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
> - PHY0_TEST_WREN | PHY0_TEST_RST;
> - else
> - val = (data & PHY_TEST_DATA) |
> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
> - ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
> - PHY1_TEST_WREN | PHY1_TEST_RST;
> - writel(val, reg);
> -
> - value = val;
> - if (priv->type == PHY_TYPE_0)
> - value |= PHY0_TEST_CLK;
> - else
> - value |= PHY1_TEST_CLK;
> - writel(value, reg);
> -
> - writel(val, reg);
> + if (priv->ports[port].base)
> + /* FIXME: fill stride in priv */
when?
> + writel(data, (u32 *)priv->ports[port].base + addr);
> + else {
> + if (priv->type == PHY_TYPE_0)
> + val = (data & PHY_TEST_DATA) |
> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
> + ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
> + PHY0_TEST_WREN | PHY0_TEST_RST;
> + else
> + val = (data & PHY_TEST_DATA) |
> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
> + ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
> + PHY1_TEST_WREN | PHY1_TEST_RST;
> + writel(val, reg);
> +
> + value = val;
> + if (priv->type == PHY_TYPE_0)
> + value |= PHY0_TEST_CLK;
> + else
> + value |= PHY1_TEST_CLK;
> + writel(value, reg);
> +
> + writel(val, reg);
val and value are very helpful variables, do consider naming them
better!
> + }
> }
>
> static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
> @@ -104,7 +112,7 @@ static int hisi_inno_phy_init(struct phy *phy)
> return ret;
> udelay(REF_CLK_STABLE_TIME);
>
> - reset_control_deassert(priv->por_rst);
> + reset_control_deassert(priv->rsts);
> udelay(POR_RST_COMPLETE_TIME);
>
> /* Set up phy registers */
> @@ -122,7 +130,7 @@ static int hisi_inno_phy_exit(struct phy *phy)
> struct hisi_inno_phy_priv *priv = port->priv;
>
> reset_control_assert(port->utmi_rst);
> - reset_control_assert(priv->por_rst);
> + reset_control_assert(priv->rsts);
> clk_disable_unprepare(priv->ref_clk);
>
> return 0;
> @@ -158,15 +166,16 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
> if (IS_ERR(priv->ref_clk))
> return PTR_ERR(priv->ref_clk);
>
> - priv->por_rst = devm_reset_control_get_exclusive(dev, NULL);
> - if (IS_ERR(priv->por_rst))
> - return PTR_ERR(priv->por_rst);
> + priv->rsts = devm_reset_control_array_get_exclusive(dev);
> + if (IS_ERR(priv->rsts))
> + return PTR_ERR(priv->rsts);
>
> priv->type = (uintptr_t) of_device_get_match_data(dev);
>
> for_each_child_of_node(np, child) {
> struct reset_control *rst;
> struct phy *phy;
> + void __iomem *base;
>
> rst = of_reset_control_get_exclusive(child, NULL);
> if (IS_ERR(rst)) {
> @@ -174,7 +183,10 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
> return PTR_ERR(rst);
> }
>
> + base = of_iomap(child, 0);
> +
> priv->ports[i].utmi_rst = rst;
> + priv->ports[i].base = base;
> priv->ports[i].priv = priv;
>
> phy = devm_phy_create(dev, child, &hisi_inno_phy_ops);
> @@ -205,6 +217,8 @@ static const struct of_device_id hisi_inno_phy_of_match[] = {
> .data = (void *) PHY_TYPE_0 },
> { .compatible = "hisilicon,hi3798mv100-usb2-phy",
> .data = (void *) PHY_TYPE_1 },
> + { .compatible = "hisilicon,hi3798mv200-usb2-phy",
> + .data = (void *) PHY_TYPE_MMIO },
> { },
> };
> MODULE_DEVICE_TABLE(of, hisi_inno_phy_of_match);
>
> --
> 2.43.0
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
From: Krzysztof Kozlowski @ 2024-04-05 17:08 UTC (permalink / raw)
To: Mithil
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
In-Reply-To: <CAGzNGRktm5gMj=bhtX2RAzcn1v5ref+nV-HV3Fct56FzAzxjWA@mail.gmail.com>
On 05/04/2024 18:29, Mithil wrote:
> On Fri, Apr 5, 2024 at 9:27 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 05/04/2024 16:48, Mithil wrote:
>>> So sorry about the 2nd patch being sent as a new mail, here is a new
>>> patch with the changes as suggested
>>>
>>>> Please use subject prefixes matching the subsystem
>>> Changed the patch name to match the folder history.
>>
>> Nothing improved. What the history tells you?
>>
>
> Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema"
> Not really sure what else I should change.
But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to
DT schema"?
Where is the ASoC?
>
>>>
>>>> Is it your full name?
>>> Fixed it, my apologies.
>>>
>>>> Filename like compatible.
>>> Fixed.
>>
>> Still not, compatible is omap4.
>>
>
> Sorry, seems like I was sending the old file again.
> Will fix this.
>
>>>
>>>> Please open existing bindings and look how it is done there.
>>> Changed it, is it fine now?
>>
>> You mean v2? I have no clue to what you are responding here, but no, v2
>> did not improve much.
>>
>
> Again, could you guide me to what needs to be done?
> Description for reg should be fine as this is how it is done in other
> files as well.
reg is not correct. Please point me to files doing that way, so I can
fix them.
You need items with description.
> Interrupts and hwmods use maxItems now.
hwmods lost description, why?
> Changed nodename to be generic in example as well.
"mcpdm" does not feel generic. What is mcpdm? Google finds nothing.
Maybe just "pdm"?
Anyway, this patch has exactly the same name as v1, so what did you
improve? Your v2 is almost the same as v1.
> Those were the suggested changes previously.
Best regards,
Krzysztof
^ permalink raw reply
* Re: (subset) [PATCH v2 0/7] arm64: qcom-sm8[456]50: properly describe the PCIe Gen4x2 PHY AUX clock
From: Vinod Koul @ 2024-04-05 17:09 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel,
Krzysztof Kozlowski, Dmitry Baryshkov
In-Reply-To: <20240322-topic-sm8x50-upstream-pcie-1-phy-aux-clk-v2-0-3ec0a966d52f@linaro.org>
On Fri, 22 Mar 2024 10:42:37 +0100, Neil Armstrong wrote:
> The PCIe Gen4x2 PHY found in the SM8[456]50 SoCs have a second clock named
> "PHY_AUX_CLK" which is an input of the Global Clock Controller (GCC) which
> is muxed & gated then returned to the PHY as an input.
>
> Document the clock IDs to select the PIPE clock or the AUX clock,
> also enforce a second clock-output-names and a #clock-cells value of 1
> for the PCIe Gen4x2 PHY found in the SM8[456]50 SoCs.
>
> [...]
Applied, thanks!
[1/7] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: document PHY AUX clock on SM8[456]50 SoCs
commit: 72bea132f3680ee51e7ed2cee62892b6f5121909
[2/7] phy: qcom: qmp-pcie: refactor clock register code
commit: 677b45114b4430a43d2602296617efc4d3f2ab7a
[3/7] phy: qcom: qmp-pcie: register second optional PHY AUX clock
commit: 583ca9ccfa806605ae1391aafa3f78a8a2cc0b48
[4/7] phy: qcom: qmp-pcie: register PHY AUX clock for SM8[456]50 4x2 PCIe PHY
commit: 5cee04a8369049b92d52995e320abff18dfeda44
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add ft5426
From: Andy Shevchenko @ 2024-04-05 17:21 UTC (permalink / raw)
To: Andreas Kemnade
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
o.rempel, u.kleine-koenig, hdegoede, ye.xingchen, p.puschmann,
linux-input, devicetree, linux-kernel, caleb.connolly
In-Reply-To: <20240405182832.4e457695@aktux>
On Fri, Apr 5, 2024 at 7:28 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> On Fri, 5 Apr 2024 18:13:45 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Apr 5, 2024 at 1:20 AM Andreas Kemnade <andreas@kemnade.info> wrote:
...
> > > @@ -1484,6 +1484,7 @@ static const struct of_device_id edt_ft5x06_of_match[] = {
> > > { .compatible = "edt,edt-ft5206", .data = &edt_ft5x06_data },
> > > { .compatible = "edt,edt-ft5306", .data = &edt_ft5x06_data },
> > > { .compatible = "edt,edt-ft5406", .data = &edt_ft5x06_data },
> > > + { .compatible = "focaltech,ft5426", .data = &edt_ft5506_data },
> >
> > Why a different vendor prefix?
> > In case you need to use this one, keep the list sorted, currently this
> > splits the edt,* ones.
> >
> How do I know whether to use evervision or edt instead?
Ask DT people, the vendor-prefixes lists both...
> I sorted by the numbers. Looking at datasheets for other controllers I see
> https://www.displayfuture.com/Display/datasheet/controller/FT5x06.pdf
> it only mentions FocalTech Systems Co., Ltd.
But does the driver use that? AFAICS it uses edt. Perhaps it's due to
a business split, not to my knowledge anyway.
> So how the vendor prefixes are derived?
Rob, Krzysztof?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
From: Mithil @ 2024-04-05 17:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
In-Reply-To: <c9084453-65f1-43b0-88df-5b73052ccb72@linaro.org>
On Fri, Apr 5, 2024 at 10:38 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 05/04/2024 18:29, Mithil wrote:
> > On Fri, Apr 5, 2024 at 9:27 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 05/04/2024 16:48, Mithil wrote:
> >>> So sorry about the 2nd patch being sent as a new mail, here is a new
> >>> patch with the changes as suggested
> >>>
> >>>> Please use subject prefixes matching the subsystem
> >>> Changed the patch name to match the folder history.
> >>
> >> Nothing improved. What the history tells you?
> >>
> >
> > Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema"
> > Not really sure what else I should change.
>
> But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to
> DT schema"?
>
> Where is the ASoC?
>
I did change it, will send the patch again.
>
> reg is not correct. Please point me to files doing that way, so I can
> fix them.
>
> You need items with description.
>
Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
I referred here for the description, but will add items for the 2 regs
> > Interrupts and hwmods use maxItems now.
>
> hwmods lost description, why?
Seems self explanatory.
> > Changed nodename to be generic in example as well.
>
> "mcpdm" does not feel generic. What is mcpdm? Google finds nothing.
> Maybe just "pdm"?
>
Multichannel PDM Controller. Kept it like that since the node is also
called as mcpdm in the devicetree. Calling it pdm might cause
confusion.
Best Regards,
Mithil
^ permalink raw reply
* Re: [PATCH 0/5] Add parsing for Zimop ISA extension
From: Deepak Gupta @ 2024-04-05 17:33 UTC (permalink / raw)
To: Andrew Jones
Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Conor Dooley, Rob Herring,
Krzysztof Kozlowski, Anup Patel, Shuah Khan, Atish Patra,
linux-doc, linux-riscv, linux-kernel, devicetree, kvm, kvm-riscv,
linux-kselftest
In-Reply-To: <20240405-091c6c174f023d74b434059d@orel>
On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote:
> > The Zimop ISA extension was ratified recently. This series adds support
> > for parsing it from riscv,isa, hwprobe export and kvm support for
> > Guest/VM.
>
> I'm not sure we need this. Zimop by itself isn't useful, so I don't know
> if we need to advertise it at all. When an extension comes along that
> redefines some MOPs, then we'll advertise that extension, but the fact
> Zimop is used for that extension is really just an implementation detail.
Only situation I see this can be useful is this:--
An implementer, implemented Zimops in CPU solely for the purpose that they can
run mainline distro & packages on their hardware and don't want to leverage any
feature which are built on top of Zimop.
As an example zicfilp and zicfiss are dependent on zimops. glibc can
do following
1) check elf header if binary was compiled with zicfiss and zicfilp,
if yes goto step 2, else goto step 6.
2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes
goto step 5. else goto step 3
3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4
4) This binary won't be able to run successfully on this platform,
issue exit syscall. <-- termination
5) issue prctl to enable shadow stack and landing pad for current task
<-- enable feature
6) let the binary run <-- let the binary run because no harm can be done
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Mayank Rana @ 2024-04-05 17:41 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-pci, lpieralisi, kw, robh, bhelgaas, andersson,
krzysztof.kozlowski+dt, conor+dt, devicetree, linux-arm-msm,
quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
In-Reply-To: <20240405052918.GA2953@thinkpad>
Hi Mani
On 4/4/2024 10:30 PM, Manivannan Sadhasivam wrote:
> On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
>> On some of Qualcomm platform, firmware configures PCIe controller into
>> ECAM mode allowing static memory allocation for configuration space of
>> supported bus range. Firmware also takes care of bringing up PCIe PHY
>> and performing required operation to bring PCIe link into D0. Firmware
>> also manages system resources (e.g. clocks/regulators/resets/ bus voting).
>> Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
>> root complex and connected PCIe devices. Firmware won't be enumerating
>> or powering up PCIe root complex until this driver invokes power domain
>> based notification to bring PCIe link into D0/D3cold mode.
>>
>
> Is this an in-house PCIe IP of Qualcomm or the same DWC IP that is used in other
> SoCs?
>
> - Mani
Driver is validated on SA8775p-ride platform using PCIe DWC IP for
now.Although this driver doesn't need to know used PCIe controller and
PHY IP as well programming sequence as that would be taken care by firmware.
>> This driver also support MSI functionality using PCIe controller based
>> MSI controller as GIC ITS based MSI functionality is not available on
>> some of platform.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> drivers/pci/controller/Kconfig | 12 +
>> drivers/pci/controller/Makefile | 1 +
>> drivers/pci/controller/pcie-qcom-ecam.c | 575 ++++++++++++++++++++++++++++++++
>> 3 files changed, 588 insertions(+)
>> create mode 100644 drivers/pci/controller/pcie-qcom-ecam.c
>>
>> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
>> index e534c02..abbd9f2 100644
>> --- a/drivers/pci/controller/Kconfig
>> +++ b/drivers/pci/controller/Kconfig
>> @@ -353,6 +353,18 @@ config PCIE_XILINX_CPM
>> Say 'Y' here if you want kernel support for the
>> Xilinx Versal CPM host bridge.
>>
>> +config PCIE_QCOM_ECAM
>> + tristate "QCOM PCIe ECAM host controller"
>> + depends on ARCH_QCOM && PCI
>> + depends on OF
>> + select PCI_MSI
>> + select PCI_HOST_COMMON
>> + select IRQ_DOMAIN
>> + help
>> + Say 'Y' here if you want to use ECAM shift mode compatible Qualcomm
>> + PCIe root host controller. The controller is programmed using firmware
>> + to support ECAM compatible memory address space.
>> +
>> source "drivers/pci/controller/cadence/Kconfig"
>> source "drivers/pci/controller/dwc/Kconfig"
>> source "drivers/pci/controller/mobiveil/Kconfig"
>> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
>> index f2b19e6..2f1ee1e 100644
>> --- a/drivers/pci/controller/Makefile
>> +++ b/drivers/pci/controller/Makefile
>> @@ -40,6 +40,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
>> obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
>> obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
>> obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
>> +obj-$(CONFIG_PCIE_QCOM_ECAM) += pcie-qcom-ecam.o
>>
>> # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
>> obj-y += dwc/
>> diff --git a/drivers/pci/controller/pcie-qcom-ecam.c b/drivers/pci/controller/pcie-qcom-ecam.c
>> new file mode 100644
>> index 00000000..5b4c68b
>> --- /dev/null
>> +++ b/drivers/pci/controller/pcie-qcom-ecam.c
>> @@ -0,0 +1,575 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Qualcomm PCIe ECAM root host controller driver
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/irq.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/msi.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci-ecam.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +
>> +#define PCIE_MSI_CTRL_BASE (0x820)
>> +#define PCIE_MSI_CTRL_SIZE (0x68)
>> +#define PCIE_MSI_CTRL_ADDR_OFFS (0x0)
>> +#define PCIE_MSI_CTRL_UPPER_ADDR_OFFS (0x4)
>> +#define PCIE_MSI_CTRL_INT_N_EN_OFFS(n) (0x8 + 0xc * (n))
>> +#define PCIE_MSI_CTRL_INT_N_MASK_OFFS(n) (0xc + 0xc * (n))
>> +#define PCIE_MSI_CTRL_INT_N_STATUS_OFFS(n) (0x10 + 0xc * (n))
>> +
>> +#define MSI_DB_ADDR 0xa0000000
>> +#define MSI_IRQ_PER_GRP (32)
>> +
>> +/**
>> + * struct qcom_msi_irq - MSI IRQ information
>> + * @client: pointer to MSI client struct
>> + * @grp: group the irq belongs to
>> + * @grp_index: index in group
>> + * @hwirq: hwirq number
>> + * @virq: virq number
>> + * @pos: position in MSI bitmap
>> + */
>> +struct qcom_msi_irq {
>> + struct qcom_msi_client *client;
>> + struct qcom_msi_grp *grp;
>> + unsigned int grp_index;
>> + unsigned int hwirq;
>> + unsigned int virq;
>> + u32 pos;
>> +};
>> +
>> +/**
>> + * struct qcom_msi_grp - MSI group information
>> + * @int_en_reg: memory-mapped interrupt enable register address
>> + * @int_mask_reg: memory-mapped interrupt mask register address
>> + * @int_status_reg: memory-mapped interrupt status register address
>> + * @mask: tracks masked/unmasked MSI
>> + * @irqs: structure to MSI IRQ information
>> + */
>> +struct qcom_msi_grp {
>> + void __iomem *int_en_reg;
>> + void __iomem *int_mask_reg;
>> + void __iomem *int_status_reg;
>> + u32 mask;
>> + struct qcom_msi_irq irqs[MSI_IRQ_PER_GRP];
>> +};
>> +
>> +/**
>> + * struct qcom_msi - PCIe controller based MSI controller information
>> + * @clients: list for tracking clients
>> + * @dev: platform device node
>> + * @nr_hwirqs: total number of hardware IRQs
>> + * @nr_virqs: total number of virqs
>> + * @nr_grps: total number of groups
>> + * @grps: pointer to all groups information
>> + * @bitmap: tracks used/unused MSI
>> + * @mutex: for modifying MSI client list and bitmap
>> + * @inner_domain: parent domain; gen irq related
>> + * @msi_domain: child domain; pcie related
>> + * @msi_db_addr: MSI doorbell address
>> + * @cfg_lock: lock for configuring MSI controller registers
>> + * @pcie_msi_cfg: memory-mapped MSI controller register space
>> + */
>> +struct qcom_msi {
>> + struct list_head clients;
>> + struct device *dev;
>> + int nr_hwirqs;
>> + int nr_virqs;
>> + int nr_grps;
>> + struct qcom_msi_grp *grps;
>> + unsigned long *bitmap;
>> + struct mutex mutex;
>> + struct irq_domain *inner_domain;
>> + struct irq_domain *msi_domain;
>> + phys_addr_t msi_db_addr;
>> + spinlock_t cfg_lock;
>> + void __iomem *pcie_msi_cfg;
>> +};
>> +
>> +/**
>> + * struct qcom_msi_client - structure for each client of MSI controller
>> + * @node: list to track number of MSI clients
>> + * @msi: client specific MSI controller based resource pointer
>> + * @dev: client's dev of pci_dev
>> + * @nr_irqs: number of irqs allocated for client
>> + * @msi_addr: MSI doorbell address
>> + */
>> +struct qcom_msi_client {
>> + struct list_head node;
>> + struct qcom_msi *msi;
>> + struct device *dev;
>> + unsigned int nr_irqs;
>> + phys_addr_t msi_addr;
>> +};
>> +
>> +static void qcom_msi_handler(struct irq_desc *desc)
>> +{
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> + struct qcom_msi_grp *msi_grp;
>> + u32 status;
>> + int i;
>> +
>> + chained_irq_enter(chip, desc);
>> +
>> + msi_grp = irq_desc_get_handler_data(desc);
>> + status = readl_relaxed(msi_grp->int_status_reg);
>> + status ^= (msi_grp->mask & status);
>> + writel(status, msi_grp->int_status_reg);
>> +
>> + for (i = 0; status; i++, status >>= 1)
>> + if (status & 0x1)
>> + generic_handle_irq(msi_grp->irqs[i].virq);
>> +
>> + chained_irq_exit(chip, desc);
>> +}
>> +
>> +static void qcom_msi_mask_irq(struct irq_data *data)
>> +{
>> + struct irq_data *parent_data;
>> + struct qcom_msi_irq *msi_irq;
>> + struct qcom_msi_grp *msi_grp;
>> + struct qcom_msi *msi;
>> + unsigned long flags;
>> +
>> + parent_data = data->parent_data;
>> + if (!parent_data)
>> + return;
>> +
>> + msi_irq = irq_data_get_irq_chip_data(parent_data);
>> + msi = msi_irq->client->msi;
>> + msi_grp = msi_irq->grp;
>> +
>> + spin_lock_irqsave(&msi->cfg_lock, flags);
>> + pci_msi_mask_irq(data);
>> + msi_grp->mask |= BIT(msi_irq->grp_index);
>> + writel(msi_grp->mask, msi_grp->int_mask_reg);
>> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
>> +}
>> +
>> +static void qcom_msi_unmask_irq(struct irq_data *data)
>> +{
>> + struct irq_data *parent_data;
>> + struct qcom_msi_irq *msi_irq;
>> + struct qcom_msi_grp *msi_grp;
>> + struct qcom_msi *msi;
>> + unsigned long flags;
>> +
>> + parent_data = data->parent_data;
>> + if (!parent_data)
>> + return;
>> +
>> + msi_irq = irq_data_get_irq_chip_data(parent_data);
>> + msi = msi_irq->client->msi;
>> + msi_grp = msi_irq->grp;
>> +
>> + spin_lock_irqsave(&msi->cfg_lock, flags);
>> + msi_grp->mask &= ~BIT(msi_irq->grp_index);
>> + writel(msi_grp->mask, msi_grp->int_mask_reg);
>> + pci_msi_unmask_irq(data);
>> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
>> +}
>> +
>> +static struct irq_chip qcom_msi_irq_chip = {
>> + .name = "qcom_pci_msi",
>> + .irq_enable = qcom_msi_unmask_irq,
>> + .irq_disable = qcom_msi_mask_irq,
>> + .irq_mask = qcom_msi_mask_irq,
>> + .irq_unmask = qcom_msi_unmask_irq,
>> +};
>> +
>> +static int qcom_msi_domain_prepare(struct irq_domain *domain, struct device *dev,
>> + int nvec, msi_alloc_info_t *arg)
>> +{
>> + struct qcom_msi *msi = domain->parent->host_data;
>> + struct qcom_msi_client *client;
>> +
>> + client = kzalloc(sizeof(*client), GFP_KERNEL);
>> + if (!client)
>> + return -ENOMEM;
>> +
>> + client->msi = msi;
>> + client->dev = dev;
>> + client->msi_addr = msi->msi_db_addr;
>> + mutex_lock(&msi->mutex);
>> + list_add_tail(&client->node, &msi->clients);
>> + mutex_unlock(&msi->mutex);
>> +
>> + /* zero out struct for pcie msi framework */
>> + memset(arg, 0, sizeof(*arg));
>> + return 0;
>> +}
>> +
>> +static struct msi_domain_ops qcom_msi_domain_ops = {
>> + .msi_prepare = qcom_msi_domain_prepare,
>> +};
>> +
>> +static struct msi_domain_info qcom_msi_domain_info = {
>> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
>> + MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
>> + .ops = &qcom_msi_domain_ops,
>> + .chip = &qcom_msi_irq_chip,
>> +};
>> +
>> +static int qcom_msi_irq_set_affinity(struct irq_data *data,
>> + const struct cpumask *mask, bool force)
>> +{
>> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
>> + int ret = 0;
>> +
>> + if (!parent_data)
>> + return -ENODEV;
>> +
>> + /* set affinity for MSI HW IRQ */
>> + if (parent_data->chip->irq_set_affinity)
>> + ret = parent_data->chip->irq_set_affinity(parent_data, mask, force);
>> +
>> + return ret;
>> +}
>> +
>> +static void qcom_msi_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>> +{
>> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
>> + struct qcom_msi_irq *msi_irq = irq_data_get_irq_chip_data(data);
>> + struct qcom_msi_client *client = msi_irq->client;
>> +
>> + if (!parent_data)
>> + return;
>> +
>> + msg->address_lo = lower_32_bits(client->msi_addr);
>> + msg->address_hi = upper_32_bits(client->msi_addr);
>> + msg->data = msi_irq->pos;
>> +}
>> +
>> +static struct irq_chip qcom_msi_bottom_irq_chip = {
>> + .name = "qcom_msi",
>> + .irq_set_affinity = qcom_msi_irq_set_affinity,
>> + .irq_compose_msi_msg = qcom_msi_irq_compose_msi_msg,
>> +};
>> +
>> +static int qcom_msi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> + unsigned int nr_irqs, void *args)
>> +{
>> + struct device *dev = ((msi_alloc_info_t *)args)->desc->dev;
>> + struct qcom_msi_client *tmp, *client = NULL;
>> + struct qcom_msi *msi = domain->host_data;
>> + int i, ret = 0;
>> + int pos;
>> +
>> + mutex_lock(&msi->mutex);
>> + list_for_each_entry(tmp, &msi->clients, node) {
>> + if (tmp->dev == dev) {
>> + client = tmp;
>> + break;
>> + }
>> + }
>> +
>> + if (!client) {
>> + dev_err(msi->dev, "failed to find MSI client dev\n");
>> + ret = -ENODEV;
>> + goto out;
>> + }
>> +
>> + pos = bitmap_find_next_zero_area(msi->bitmap, msi->nr_virqs, 0,
>> + nr_irqs, nr_irqs - 1);
>> + if (pos > msi->nr_virqs) {
>> + ret = -ENOSPC;
>> + goto out;
>> + }
>> +
>> + bitmap_set(msi->bitmap, pos, nr_irqs);
>> + for (i = 0; i < nr_irqs; i++) {
>> + u32 grp = pos / MSI_IRQ_PER_GRP;
>> + u32 index = pos % MSI_IRQ_PER_GRP;
>> + struct qcom_msi_irq *msi_irq = &msi->grps[grp].irqs[index];
>> +
>> + msi_irq->virq = virq + i;
>> + msi_irq->client = client;
>> + irq_domain_set_info(domain, msi_irq->virq,
>> + msi_irq->hwirq,
>> + &qcom_msi_bottom_irq_chip, msi_irq,
>> + handle_simple_irq, NULL, NULL);
>> + client->nr_irqs++;
>> + pos++;
>> + }
>> +out:
>> + mutex_unlock(&msi->mutex);
>> + return ret;
>> +}
>> +
>> +static void qcom_msi_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>> + unsigned int nr_irqs)
>> +{
>> + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
>> + struct qcom_msi_client *client;
>> + struct qcom_msi_irq *msi_irq;
>> + struct qcom_msi *msi;
>> +
>> + if (!data)
>> + return;
>> +
>> + msi_irq = irq_data_get_irq_chip_data(data);
>> + client = msi_irq->client;
>> + msi = client->msi;
>> +
>> + mutex_lock(&msi->mutex);
>> + bitmap_clear(msi->bitmap, msi_irq->pos, nr_irqs);
>> +
>> + client->nr_irqs -= nr_irqs;
>> + if (!client->nr_irqs) {
>> + list_del(&client->node);
>> + kfree(client);
>> + }
>> + mutex_unlock(&msi->mutex);
>> +
>> + irq_domain_free_irqs_parent(domain, virq, nr_irqs);
>> +}
>> +
>> +static const struct irq_domain_ops msi_domain_ops = {
>> + .alloc = qcom_msi_irq_domain_alloc,
>> + .free = qcom_msi_irq_domain_free,
>> +};
>> +
>> +static int qcom_msi_alloc_domains(struct qcom_msi *msi)
>> +{
>> + msi->inner_domain = irq_domain_add_linear(NULL, msi->nr_virqs,
>> + &msi_domain_ops, msi);
>> + if (!msi->inner_domain) {
>> + dev_err(msi->dev, "failed to create IRQ inner domain\n");
>> + return -ENOMEM;
>> + }
>> +
>> + msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->dev->of_node),
>> + &qcom_msi_domain_info, msi->inner_domain);
>> + if (!msi->msi_domain) {
>> + dev_err(msi->dev, "failed to create MSI domain\n");
>> + irq_domain_remove(msi->inner_domain);
>> + return -ENOMEM;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_msi_irq_setup(struct qcom_msi *msi)
>> +{
>> + struct qcom_msi_grp *msi_grp;
>> + struct qcom_msi_irq *msi_irq;
>> + int i, index, ret;
>> + unsigned int irq;
>> +
>> + /* setup each MSI group. nr_hwirqs == nr_grps */
>> + for (i = 0; i < msi->nr_hwirqs; i++) {
>> + irq = irq_of_parse_and_map(msi->dev->of_node, i);
>> + if (!irq) {
>> + dev_err(msi->dev,
>> + "MSI: failed to parse/map interrupt\n");
>> + ret = -ENODEV;
>> + goto free_irqs;
>> + }
>> +
>> + msi_grp = &msi->grps[i];
>> + msi_grp->int_en_reg = msi->pcie_msi_cfg +
>> + PCIE_MSI_CTRL_INT_N_EN_OFFS(i);
>> + msi_grp->int_mask_reg = msi->pcie_msi_cfg +
>> + PCIE_MSI_CTRL_INT_N_MASK_OFFS(i);
>> + msi_grp->int_status_reg = msi->pcie_msi_cfg +
>> + PCIE_MSI_CTRL_INT_N_STATUS_OFFS(i);
>> +
>> + for (index = 0; index < MSI_IRQ_PER_GRP; index++) {
>> + msi_irq = &msi_grp->irqs[index];
>> +
>> + msi_irq->grp = msi_grp;
>> + msi_irq->grp_index = index;
>> + msi_irq->pos = (i * MSI_IRQ_PER_GRP) + index;
>> + msi_irq->hwirq = irq;
>> + }
>> +
>> + irq_set_chained_handler_and_data(irq, qcom_msi_handler, msi_grp);
>> + }
>> +
>> + return 0;
>> +
>> +free_irqs:
>> + for (--i; i >= 0; i--) {
>> + irq = msi->grps[i].irqs[0].hwirq;
>> +
>> + irq_set_chained_handler_and_data(irq, NULL, NULL);
>> + irq_dispose_mapping(irq);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void qcom_msi_config(struct irq_domain *domain)
>> +{
>> + struct qcom_msi *msi;
>> + int i;
>> +
>> + msi = domain->parent->host_data;
>> +
>> + /* program termination address */
>> + writel(msi->msi_db_addr, msi->pcie_msi_cfg + PCIE_MSI_CTRL_ADDR_OFFS);
>> + writel(0, msi->pcie_msi_cfg + PCIE_MSI_CTRL_UPPER_ADDR_OFFS);
>> +
>> + /* restore mask and enable all interrupts for each group */
>> + for (i = 0; i < msi->nr_grps; i++) {
>> + struct qcom_msi_grp *msi_grp = &msi->grps[i];
>> +
>> + writel(msi_grp->mask, msi_grp->int_mask_reg);
>> + writel(~0, msi_grp->int_en_reg);
>> + }
>> +}
>> +
>> +static void qcom_msi_deinit(struct qcom_msi *msi)
>> +{
>> + irq_domain_remove(msi->msi_domain);
>> + irq_domain_remove(msi->inner_domain);
>> +}
>> +
>> +static struct qcom_msi *qcom_msi_init(struct device *dev)
>> +{
>> + struct qcom_msi *msi;
>> + u64 addr;
>> + int ret;
>> +
>> + msi = devm_kzalloc(dev, sizeof(*msi), GFP_KERNEL);
>> + if (!msi)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + msi->dev = dev;
>> + mutex_init(&msi->mutex);
>> + spin_lock_init(&msi->cfg_lock);
>> + INIT_LIST_HEAD(&msi->clients);
>> +
>> + msi->msi_db_addr = MSI_DB_ADDR;
>> + msi->nr_hwirqs = of_irq_count(dev->of_node);
>> + if (!msi->nr_hwirqs) {
>> + dev_err(msi->dev, "no hwirqs found\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
>> + dev_err(msi->dev, "failed to get reg address\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + dev_dbg(msi->dev, "hwirq:%d pcie_msi_cfg:%llx\n", msi->nr_hwirqs, addr);
>> + msi->pcie_msi_cfg = devm_ioremap(dev, addr + PCIE_MSI_CTRL_BASE, PCIE_MSI_CTRL_SIZE);
>> + if (!msi->pcie_msi_cfg)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + msi->nr_virqs = msi->nr_hwirqs * MSI_IRQ_PER_GRP;
>> + msi->nr_grps = msi->nr_hwirqs;
>> + msi->grps = devm_kcalloc(dev, msi->nr_grps, sizeof(*msi->grps), GFP_KERNEL);
>> + if (!msi->grps)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + msi->bitmap = devm_kcalloc(dev, BITS_TO_LONGS(msi->nr_virqs),
>> + sizeof(*msi->bitmap), GFP_KERNEL);
>> + if (!msi->bitmap)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + ret = qcom_msi_alloc_domains(msi);
>> + if (ret)
>> + return ERR_PTR(ret);
>> +
>> + ret = qcom_msi_irq_setup(msi);
>> + if (ret) {
>> + qcom_msi_deinit(msi);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + qcom_msi_config(msi->msi_domain);
>> + return msi;
>> +}
>> +
>> +static int qcom_pcie_ecam_suspend_noirq(struct device *dev)
>> +{
>> + return pm_runtime_put_sync(dev);
>> +}
>> +
>> +static int qcom_pcie_ecam_resume_noirq(struct device *dev)
>> +{
>> + return pm_runtime_get_sync(dev);
>> +}
>> +
>> +static int qcom_pcie_ecam_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct qcom_msi *msi;
>> + int ret;
>> +
>> + ret = devm_pm_runtime_enable(dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = pm_runtime_resume_and_get(dev);
>> + if (ret < 0) {
>> + dev_err(dev, "fail to enable pcie controller: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + msi = qcom_msi_init(dev);
>> + if (IS_ERR(msi)) {
>> + pm_runtime_put_sync(dev);
>> + return PTR_ERR(msi);
>> + }
>> +
>> + ret = pci_host_common_probe(pdev);
>> + if (ret) {
>> + dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);
>> + qcom_msi_deinit(msi);
>> + pm_runtime_put_sync(dev);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static const struct dev_pm_ops qcom_pcie_ecam_pm_ops = {
>> + NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_ecam_suspend_noirq,
>> + qcom_pcie_ecam_resume_noirq)
>> +};
>> +
>> +static const struct pci_ecam_ops qcom_pcie_ecam_ops = {
>> + .pci_ops = {
>> + .map_bus = pci_ecam_map_bus,
>> + .read = pci_generic_config_read,
>> + .write = pci_generic_config_write,
>> + }
>> +};
>> +
>> +static const struct of_device_id qcom_pcie_ecam_of_match[] = {
>> + {
>> + .compatible = "qcom,pcie-ecam-rc",
>> + .data = &qcom_pcie_ecam_ops,
>> + },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_pcie_ecam_of_match);
>> +
>> +static struct platform_driver qcom_pcie_ecam_driver = {
>> + .probe = qcom_pcie_ecam_probe,
>> + .driver = {
>> + .name = "qcom-pcie-ecam-rc",
>> + .suppress_bind_attrs = true,
>> + .of_match_table = qcom_pcie_ecam_of_match,
>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> + .pm = &qcom_pcie_ecam_pm_ops,
>> + },
>> +};
>> +module_platform_driver(qcom_pcie_ecam_driver);
>> +
>> +MODULE_DESCRIPTION("Qualcomm PCIe ECAM root complex driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.7.4
>>
>
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add Qualcomm PCIe ECAM root complex driver
From: Mayank Rana @ 2024-04-05 17:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-pci, lpieralisi, kw, robh, bhelgaas,
andersson, manivannan.sadhasivam, krzysztof.kozlowski+dt,
conor+dt, devicetree
Cc: linux-arm-msm, quic_ramkri, quic_nkela, quic_shazhuss,
quic_msarkar, quic_nitegupt
In-Reply-To: <ab967c4c-363b-4530-b11e-6de7f3fa0426@linaro.org>
Hi Krzysztof
On 4/4/2024 11:50 PM, Krzysztof Kozlowski wrote:
> On 05/04/2024 01:02, Mayank Rana wrote:
>> Hi Krzysztof
>>
>> On 4/4/2024 12:33 PM, Krzysztof Kozlowski wrote:
>>> On 04/04/2024 21:11, Mayank Rana wrote:
>>>> On some of Qualcomm platform, firmware takes care of system resources
>>>> related to PCIe PHY and controller as well bringing up PCIe link and
>>>> having static iATU configuration for PCIe controller to work into
>>>> ECAM compliant mode. Hence add Qualcomm PCIe ECAM root complex driver.
>>>>
>>>> Tested:
>>>> - Validated NVME functionality with PCIe0 and PCIe1 on SA877p-ride platform
>>>>
>>>
>>> RFC means code is not ready, right? Please get internal review done and
>>> send it when it is ready. I am not sure if you expect any reviews. Some
>>> people send RFC and do not expect reviews. Some expect. I have no clue
>>> and I do not want to waste my time. Please clarify what you expect from
>>> maintainers regarding this contribution.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> Thanks for initial comments.
>> yes, this is work in progress. There are still more functionalities
>> planned to be added as part of this driver. Although purpose of sending
>> initial change here to get feedback and review comments in terms of
>> usage of generic Qualcomm PCIe ECAM driver, and usage of MSI
>> functionality with it. I missed mentioning this as part of cover letter.
>> So please help to review and provide feedback.
>
> Thanks for explanation. Work in progress as not ready to be merged? Then
> I am sorry, I am not going to provide review of unfinished work. I have
> many more *finished* patches to review first. You can help with these
> too....
>
> Best regards,
> Krzysztof
Ok. I am looking forward to send finished work on this once ready.
Thank you.
Regards,
Mayank
^ permalink raw reply
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
From: Yang Xiwen @ 2024-04-05 17:53 UTC (permalink / raw)
To: Vinod Koul
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
In-Reply-To: <ZhAsXUjifTD6HeKx@matsya>
On 4/6/2024 12:52 AM, Vinod Koul wrote:
> On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
> That is quite an email id!
>
>> Direct MMIO resgiter access is used by Hi3798MV200. For other models,
>> of_iomap() returns NULL due to insufficient length. So they are
> so how is that fixed... Pls describe the change...
The commit log will be rewritten in next revision. I'll try to emphasize
the PHY and its configuration interface briefly. Though i don't have
access to the datasheets and TRM so most things can not be verified.
For CV200 and MV100 INNO PHY, the configuration interface is attached to
PERICTRL(Peripheral Control Block). So we just use a register called
PERI_USB3 to configure the PHY. The bus reset, clock are all controlled
in PERI_USB3 register. To read/write to a register of the PHY, a special
sequence of register writes and reads are needed, which was implemented
in this driver.
But for MV200 INNO PHY, the configuration interface is attached directly
to system bus(MMIO). The bus clocks and resets are controlled via Clock
Reset Generator(CRG). Now we have to control them with the help of linux
clk and reset framework because they are provided by other modules.
>> unaffected.
>>
>> Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
>> switch to reset_control_array_*() APIs for that.
The commit msg is misleading here. There is no extra reset actually. The
reset also exist for existing users. The initial author just decided to
manage it in the hisi_inno_phy_write_reg() routine(without using
reset_control_* APIs) and omit it in the binding.
> That probably should be a different patch
I guess so. From my point of view, the whole patch is to introduce the
support for Hi3798MV200 variant of the INNO PHY. So i've decided to
squash the two changes into one single commit.
>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>> drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 66 ++++++++++++++++++------------
>> 1 file changed, 40 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
>> index b7e740eb4752..df154cd99ed8 100644
>> --- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
>> +++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
>> @@ -10,6 +10,7 @@
>> #include <linux/io.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> +#include <linux/of_address.h>
>> #include <linux/phy/phy.h>
>> #include <linux/platform_device.h>
>> #include <linux/reset.h>
>> @@ -24,6 +25,7 @@
>>
>> #define PHY_TYPE_0 0
>> #define PHY_TYPE_1 1
>> +#define PHY_TYPE_MMIO 2
>>
>> #define PHY_TEST_DATA GENMASK(7, 0)
>> #define PHY_TEST_ADDR_OFFSET 8
>> @@ -43,6 +45,7 @@
>> #define PHY_CLK_ENABLE BIT(2)
>>
>> struct hisi_inno_phy_port {
>> + void __iomem *base;
>> struct reset_control *utmi_rst;
>> struct hisi_inno_phy_priv *priv;
>> };
>> @@ -50,7 +53,7 @@ struct hisi_inno_phy_port {
>> struct hisi_inno_phy_priv {
>> void __iomem *mmio;
>> struct clk *ref_clk;
>> - struct reset_control *por_rst;
>> + struct reset_control *rsts;
>> unsigned int type;
>> struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
>> };
>> @@ -62,26 +65,31 @@ static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
>> u32 val;
>> u32 value;
>>
>> - if (priv->type == PHY_TYPE_0)
>> - val = (data & PHY_TEST_DATA) |
>> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
>> - ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
>> - PHY0_TEST_WREN | PHY0_TEST_RST;
>> - else
>> - val = (data & PHY_TEST_DATA) |
>> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
>> - ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
>> - PHY1_TEST_WREN | PHY1_TEST_RST;
>> - writel(val, reg);
>> -
>> - value = val;
>> - if (priv->type == PHY_TYPE_0)
>> - value |= PHY0_TEST_CLK;
>> - else
>> - value |= PHY1_TEST_CLK;
>> - writel(value, reg);
>> -
>> - writel(val, reg);
>> + if (priv->ports[port].base)
>> + /* FIXME: fill stride in priv */
> when?
I'm not sure. Maybe until some other users with stride other than 3? I
don't have much knowledge about other SoCs.
Maybe replace the FIXME here with some additional information.
>
>> + writel(data, (u32 *)priv->ports[port].base + addr);
>> + else {
>> + if (priv->type == PHY_TYPE_0)
>> + val = (data & PHY_TEST_DATA) |
>> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
>> + ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
>> + PHY0_TEST_WREN | PHY0_TEST_RST;
>> + else
>> + val = (data & PHY_TEST_DATA) |
>> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
>> + ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
>> + PHY1_TEST_WREN | PHY1_TEST_RST;
>> + writel(val, reg);
>> +
>> + value = val;
>> + if (priv->type == PHY_TYPE_0)
>> + value |= PHY0_TEST_CLK;
>> + else
>> + value |= PHY1_TEST_CLK;
>> + writel(value, reg);
>> +
>> + writel(val, reg);
> val and value are very helpful variables, do consider naming them
> better!
I'll consider renaming them in the next revision. Maybe val and val2?
They are just some temp vars to store register values.
>
>> + }
>> }
>>
>> static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
>> @@ -104,7 +112,7 @@ static int hisi_inno_phy_init(struct phy *phy)
>> return ret;
>> udelay(REF_CLK_STABLE_TIME);
>>
>> - reset_control_deassert(priv->por_rst);
>> + reset_control_deassert(priv->rsts);
>> udelay(POR_RST_COMPLETE_TIME);
>>
>> /* Set up phy registers */
>> @@ -122,7 +130,7 @@ static int hisi_inno_phy_exit(struct phy *phy)
>> struct hisi_inno_phy_priv *priv = port->priv;
>>
>> reset_control_assert(port->utmi_rst);
>> - reset_control_assert(priv->por_rst);
>> + reset_control_assert(priv->rsts);
>> clk_disable_unprepare(priv->ref_clk);
>>
>> return 0;
>> @@ -158,15 +166,16 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
>> if (IS_ERR(priv->ref_clk))
>> return PTR_ERR(priv->ref_clk);
>>
>> - priv->por_rst = devm_reset_control_get_exclusive(dev, NULL);
>> - if (IS_ERR(priv->por_rst))
>> - return PTR_ERR(priv->por_rst);
>> + priv->rsts = devm_reset_control_array_get_exclusive(dev);
>> + if (IS_ERR(priv->rsts))
>> + return PTR_ERR(priv->rsts);
>>
>> priv->type = (uintptr_t) of_device_get_match_data(dev);
>>
>> for_each_child_of_node(np, child) {
>> struct reset_control *rst;
>> struct phy *phy;
>> + void __iomem *base;
>>
>> rst = of_reset_control_get_exclusive(child, NULL);
>> if (IS_ERR(rst)) {
>> @@ -174,7 +183,10 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
>> return PTR_ERR(rst);
>> }
>>
>> + base = of_iomap(child, 0);
>> +
>> priv->ports[i].utmi_rst = rst;
>> + priv->ports[i].base = base;
>> priv->ports[i].priv = priv;
>>
>> phy = devm_phy_create(dev, child, &hisi_inno_phy_ops);
>> @@ -205,6 +217,8 @@ static const struct of_device_id hisi_inno_phy_of_match[] = {
>> .data = (void *) PHY_TYPE_0 },
>> { .compatible = "hisilicon,hi3798mv100-usb2-phy",
>> .data = (void *) PHY_TYPE_1 },
>> + { .compatible = "hisilicon,hi3798mv200-usb2-phy",
>> + .data = (void *) PHY_TYPE_MMIO },
>> { },
>> };
>> MODULE_DEVICE_TABLE(of, hisi_inno_phy_of_match);
>>
>> --
>> 2.43.0
--
Regards,
Yang Xiwen
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
From: Krzysztof Kozlowski @ 2024-04-05 18:19 UTC (permalink / raw)
To: Mithil
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
In-Reply-To: <CAGzNGR=2-us8GRB3RNi4_24QZ9rNBC7Lx0PFsWwbvxuRKk5ngw@mail.gmail.com>
On 05/04/2024 19:21, Mithil wrote:
> On Fri, Apr 5, 2024 at 10:38 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 05/04/2024 18:29, Mithil wrote:
>>> On Fri, Apr 5, 2024 at 9:27 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 05/04/2024 16:48, Mithil wrote:
>>>>> So sorry about the 2nd patch being sent as a new mail, here is a new
>>>>> patch with the changes as suggested
>>>>>
>>>>>> Please use subject prefixes matching the subsystem
>>>>> Changed the patch name to match the folder history.
>>>>
>>>> Nothing improved. What the history tells you?
>>>>
>>>
>>> Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema"
>>> Not really sure what else I should change.
>>
>> But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to
>> DT schema"?
>>
>> Where is the ASoC?
>>
> I did change it, will send the patch again.
>
>>
>> reg is not correct. Please point me to files doing that way, so I can
>> fix them.
>>
>> You need items with description.
>>
> Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
> I referred here for the description, but will add items for the 2 regs
I don't see at all the code you are using. It's entirely different!
Where in this file is that type of "reg" property?
>
>>> Interrupts and hwmods use maxItems now.
>>
>> hwmods lost description, why?
> Seems self explanatory.
Really? Not to me. I have no clue what this is. Also, you need
description for (almost) every non-standard, vendor property.
>
>>> Changed nodename to be generic in example as well.
>>
>> "mcpdm" does not feel generic. What is mcpdm? Google finds nothing.
>> Maybe just "pdm"?
>>
> Multichannel PDM Controller. Kept it like that since the node is also
You said you "changed nodename". So from what did you change to what?
> called as mcpdm in the devicetree. Calling it pdm might cause
Poor DTS is not the example...
> confusion.
So far I am confused. Often name of SoC block is specific, not generic.
Anyway, that's not important part, so if you claim mcpdm is generic name
of a class of devices, I am fine.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]
From: Krzysztof Kozlowski @ 2024-04-05 18:21 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
In-Reply-To: <ZhAO4YWuB8r8k+m8@lizhi-Precision-Tower-5810>
On 05/04/2024 16:46, Frank Li wrote:
> On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
>> On 04/04/2024 18:19, Frank Li wrote:
>>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
>>> i2c bus mux to connect both i2c devices. One will probe failure and other
>>> will probe success when devices driver check whoami. So one dtb can cover
>>> both board configuration.
>>
>> I don't understand it. Either you add real device or not. If one board
>> has two devices, then why do you need to check for failures?
>>
>> Anyway, don't add fake stuff to DTS.
>
> NAK can't resolve the problem. It should be common problem for long time
> cycle boards. Some chipes will be out life cycle. such as some sensor. So
> chips on boards have been replace by some pin to pin compatible sensor. For
> example:
> old boards: use sensor A with address 0x1a
> new bench: use sensor B with address 0x1b.
>
> You can treat it as two kind boards, RevA or RevB. But most user want to
> use one dtb to handle such small differences. For this case, it should be
> simple. Just add a super set.
> i2c
> {
> sensorA@1a
> {
> }
> sensorB@1b
> {
> }
> }
>
> It also depend on whoami check by i2c devices. Only A or B will probe.
>
> wm8960 and wm8962 are more complex example. wm8960 is out of life. But
> wm8962 and wm8960 have the same i2c address. The current i2c frame can't
> allow the same i2c address in one i2c bus.
>
> You are feel to NAK my method, but I hope you also provide constructive
> solution to help resolve the problem.
Yes, we resolved it long time ago. Your bootloader can (usually easily)
detect revision of the board and load appropriate DTS or DTS+DTSO.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RESEND PATCH v9 2/4] dt-bindings: stm32: update DT bingding for stm32mp25
From: Krzysztof Kozlowski @ 2024-04-05 18:22 UTC (permalink / raw)
To: Gabriel FERNANDEZ, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
Alexandre Torgue, Philipp Zabel
Cc: linux-clk, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <285f2f64-58b0-4dd0-9f1a-89306a79d572@foss.st.com>
On 05/04/2024 14:54, Gabriel FERNANDEZ wrote:
>
> On 4/5/24 09:12, Krzysztof Kozlowski wrote:
>> On 02/04/2024 14:53, gabriel.fernandez@foss.st.com wrote:
>>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>>>
>>> Now RCC driver use '.index' of clk_parent_data struct to define a parent.
>>> The majority of parents are SCMI clocks, then dt-bindings must be fixed.
>>>
>>> Fixes: b5be49db3d47 ("dt-bindings: stm32: add clocks and reset binding for stm32mp25 platform")
>> And except what Rob said, this does not look as a fix. How ABI break
>> could be a fix and what is even to fix here? Please describe the
>> observable bug, how it manifests itself and what is exactly the fix for
>> that bug.
> As I replied to Rob, there are no RCC STM32MP25 drivers already upstreamed.
>
> However, in my series, the DT binding was merged even though Stephen
> made some
>
> important remarks that needed to be taken into account.
>
> That's why I proposed a fix to update the documentation.
>
> To be sure, how would you like me to proceed?
You can send v3 and get exactly the same questions. Your commit msg must
answer to all such unusual questions. If maintainer asks something that
you need to explain, it is a hint for you that your commit msg is
inadequate.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Bjorn Helgaas @ 2024-04-05 18:30 UTC (permalink / raw)
To: Mayank Rana
Cc: linux-pci, lpieralisi, kw, robh, bhelgaas, andersson,
manivannan.sadhasivam, krzysztof.kozlowski+dt, conor+dt,
devicetree, linux-arm-msm, quic_ramkri, quic_nkela, quic_shazhuss,
quic_msarkar, quic_nitegupt
In-Reply-To: <1712257884-23841-3-git-send-email-quic_mrana@quicinc.com>
On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
> On some of Qualcomm platform, firmware configures PCIe controller into
> ECAM mode allowing static memory allocation for configuration space of
> supported bus range. Firmware also takes care of bringing up PCIe PHY
> and performing required operation to bring PCIe link into D0. Firmware
I think link state would be L0, not D0.
> also manages system resources (e.g. clocks/regulators/resets/ bus voting).
> Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
> root complex and connected PCIe devices. Firmware won't be enumerating
> or powering up PCIe root complex until this driver invokes power domain
> based notification to bring PCIe link into D0/D3cold mode.
Again.
> +config PCIE_QCOM_ECAM
> + tristate "QCOM PCIe ECAM host controller"
> + depends on ARCH_QCOM && PCI
> + depends on OF
> + select PCI_MSI
> + select PCI_HOST_COMMON
> + select IRQ_DOMAIN
> + help
> + Say 'Y' here if you want to use ECAM shift mode compatible Qualcomm
> + PCIe root host controller. The controller is programmed using firmware
> + to support ECAM compatible memory address space.
Instead of adding this at the end, place this entry so the entire list
remains sorted by vendor name.
Other related entries are "Qualcomm PCIe controller ..." (not "QCOM").
Use "ECAM PCIe controller (host mode)" (not "PCIe ECAM host
controller") so it matches similar entries.
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Does this actually work? I expected "#define dev_fmt" since you're
using dev_err(), etc below.
> +#include <linux/irqchip/chained_irq.h>
Can this be reworked so it doesn't use chained IRQs? I admit to not
being an IRQ expert, but I have the impression that it's better to
avoid the chained IRQ model when possible. See
https://lore.kernel.org/all/20231108153133.GA393726@bhelgaas/
> +#define MSI_DB_ADDR 0xa0000000
Where does this come from and why is it hard-coded here? Looks like a
magic address that maybe should come from DT?
> + * struct qcom_msi_irq - MSI IRQ information
> + * @client: pointer to MSI client struct
> + * @grp: group the irq belongs to
s/irq/IRQ/ in comments for consistency (other occurrences below).
Same for s/pcie/PCIe/ and s/msi/MSI/.
> +static void qcom_msi_mask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
Drop this test; I think it only detects logic errors in the driver or
memory corruptions, and we want to find out about those.
> +static void qcom_msi_unmask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
Drop.
> +static struct irq_chip qcom_msi_irq_chip = {
> + .name = "qcom_pci_msi",
> + .irq_enable = qcom_msi_unmask_irq,
> + .irq_disable = qcom_msi_mask_irq,
> + .irq_mask = qcom_msi_mask_irq,
> + .irq_unmask = qcom_msi_unmask_irq,
Name these so they match the struct member, e.g., the name should
contain "irq_mask", not "mask_irq") so grep finds them easily.
> +static struct msi_domain_ops qcom_msi_domain_ops = {
> + .msi_prepare = qcom_msi_domain_prepare,
Rename so function name includes the struct member name.
> +static int qcom_msi_irq_set_affinity(struct irq_data *data,
> + const struct cpumask *mask, bool force)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + int ret = 0;
> +
> + if (!parent_data)
> + return -ENODEV;
> +
> + /* set affinity for MSI HW IRQ */
Unnecessary comment.
> + if (parent_data->chip->irq_set_affinity)
> + ret = parent_data->chip->irq_set_affinity(parent_data, mask, force);
> +
> + return ret;
Drop "ret" and return directly, e.g.,
if (parent_data->chip->irq_set_affinity)
return parent_data->chip->irq_set_affinity(...);
return 0;
> +static void qcom_msi_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + struct qcom_msi_irq *msi_irq = irq_data_get_irq_chip_data(data);
> + struct qcom_msi_client *client = msi_irq->client;
> +
> + if (!parent_data)
> + return;
Drop.
> +static void qcom_msi_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> + struct qcom_msi_client *client;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi *msi;
> +
> + if (!data)
> + return;
Drop.
> +static int qcom_msi_irq_setup(struct qcom_msi *msi)
> +{
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi_irq *msi_irq;
> + int i, index, ret;
> + unsigned int irq;
> +
> + /* setup each MSI group. nr_hwirqs == nr_grps */
> + for (i = 0; i < msi->nr_hwirqs; i++) {
> + irq = irq_of_parse_and_map(msi->dev->of_node, i);
> + if (!irq) {
> + dev_err(msi->dev,
> + "MSI: failed to parse/map interrupt\n");
Possibly include "i" to identify the offending entry.
^ 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