| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aFurkudIvrbRjJ5N@shepard>
Date: Wed, 25 Jun 2025 09:56:02 +0200
From: Paul Kocialkowski <paulk@...-base.io>
To: Kuba Szczodrzyński <kuba@...zodrzynski.pl>
Cc: Maxime Ripard <mripard@...nel.org>,
Samuel Holland <samuel@...lland.org>, Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 1/5] phy: allwinner: phy-sun6i-mipi-dphy: Support LVDS in
combo D-PHY
Hi,
Thanks for working on this! See a few comments below.
On Fri 21 Feb 25, 17:17, Kuba Szczodrzyński wrote:
> Some Allwinner chips (notably the D1s/T113 and the A100) have a "combo
> MIPI DSI D-PHY" which is required when using single-link LVDS0.
>
> In this mode, the DSI peripheral is not used and the PHY is not
> configured for DSI. Instead, the COMBO_PHY_REGx registers are set to
> enable LVDS operation.
>
> Enable the PHY driver to work in LVDS mode on chips with a combo D-PHY.
>
> Signed-off-by: Kuba Szczodrzyński <kuba@...zodrzynski.pl>
> ---
> drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 65 ++++++++++++++++++++-
> 1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> index 36eab9527..f958e34da 100644
> --- a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> +++ b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> @@ -166,8 +166,8 @@
> #define SUN50I_COMBO_PHY_REG0_EN_CP BIT(0)
>
> #define SUN50I_COMBO_PHY_REG1 0x114
> -#define SUN50I_COMBO_PHY_REG2_REG_VREF1P6(n) (((n) & 0x7) << 4)
> -#define SUN50I_COMBO_PHY_REG2_REG_VREF0P8(n) ((n) & 0x7)
> +#define SUN50I_COMBO_PHY_REG1_REG_VREF1P6(n) (((n) & 0x7) << 4)
> +#define SUN50I_COMBO_PHY_REG1_REG_VREF0P8(n) ((n) & 0x7)
Good catch! Would be good to mention in the commit log (or split in a separate
patch but that might be overdoing it since this register wasn't used so far).
> #define SUN50I_COMBO_PHY_REG2 0x118
> #define SUN50I_COMBO_PHY_REG2_HS_STOP_DLY(n) ((n) & 0xff)
> @@ -181,7 +181,9 @@ struct sun6i_dphy;
>
> struct sun6i_dphy_variant {
> void (*tx_power_on)(struct sun6i_dphy *dphy);
> + void (*lvds_power_on)(struct sun6i_dphy *dphy);
> bool rx_supported;
> + bool is_combo_dphy;
> };
>
> struct sun6i_dphy {
> @@ -222,6 +224,18 @@ static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> return 0;
> }
>
> +static int sun6i_dphy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> +{
> + struct sun6i_dphy *dphy = phy_get_drvdata(phy);
> +
> + if (mode == PHY_MODE_LVDS && !dphy->variant->is_combo_dphy) {
> + /* Not a combo D-PHY: LVDS is not supported */
Missing a final . in the comment.
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static void sun6i_a31_mipi_dphy_tx_power_on(struct sun6i_dphy *dphy)
> {
> u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0);
> @@ -329,6 +343,37 @@ static void sun50i_a100_mipi_dphy_tx_power_on(struct sun6i_dphy *dphy)
> udelay(1);
> }
>
> +static void sun50i_a100_mipi_dphy_lvds_power_on(struct sun6i_dphy *dphy)
> +{
> + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG1,
> + SUN50I_COMBO_PHY_REG1_REG_VREF1P6(4) |
> + SUN50I_COMBO_PHY_REG1_REG_VREF0P8(3));
> +
> + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG0,
> + SUN50I_COMBO_PHY_REG0_EN_CP);
> + udelay(5);
Please add a white space here...
> + regmap_update_bits(dphy->regs, SUN50I_COMBO_PHY_REG0,
> + SUN50I_COMBO_PHY_REG0_EN_LVDS,
> + SUN50I_COMBO_PHY_REG0_EN_LVDS);
> + udelay(5);
here too...
> + regmap_update_bits(dphy->regs, SUN50I_COMBO_PHY_REG0,
> + SUN50I_COMBO_PHY_REG0_EN_COMBOLDO,
> + SUN50I_COMBO_PHY_REG0_EN_COMBOLDO);
> + udelay(5);
here too...
> + regmap_update_bits(dphy->regs, SUN50I_COMBO_PHY_REG0,
> + SUN50I_COMBO_PHY_REG0_EN_MIPI,
> + SUN50I_COMBO_PHY_REG0_EN_MIPI);
> +
> + regmap_write(dphy->regs, SUN6I_DPHY_ANA4_REG,
> + SUN6I_DPHY_ANA4_REG_EN_MIPI |
> + SUN6I_DPHY_ANA4_REG_IB(2));
here too...
> + regmap_write(dphy->regs, SUN6I_DPHY_ANA3_REG,
> + SUN6I_DPHY_ANA3_EN_LDOR |
> + SUN6I_DPHY_ANA3_EN_LDOD);
here too...
> + regmap_write(dphy->regs, SUN6I_DPHY_ANA2_REG, 0);
and here too in order to match the coding style.
> + regmap_write(dphy->regs, SUN6I_DPHY_ANA1_REG, 0);
> +}
> +
> static int sun6i_dphy_tx_power_on(struct sun6i_dphy *dphy)
> {
> u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0);
> @@ -492,6 +537,14 @@ static int sun6i_dphy_power_on(struct phy *phy)
> {
> struct sun6i_dphy *dphy = phy_get_drvdata(phy);
>
> + if (phy->attrs.mode == PHY_MODE_LVDS && dphy->variant->is_combo_dphy) {
> + if (dphy->variant->lvds_power_on) {
> + dphy->variant->lvds_power_on(dphy);
> + return 0;
> + }
> + return -EINVAL;
This would look better the other way round: first check:
if (!dphy->variant->lvds_power_on)
return -EINVAL;
and then call the function pointer and return 0 without extra indentation.
> + }
> +
> switch (dphy->direction) {
> case SUN6I_DPHY_DIRECTION_TX:
> return sun6i_dphy_tx_power_on(dphy);
> @@ -514,6 +567,11 @@ static int sun6i_dphy_power_off(struct phy *phy)
> regmap_write(dphy->regs, SUN6I_DPHY_ANA3_REG, 0);
> regmap_write(dphy->regs, SUN6I_DPHY_ANA4_REG, 0);
>
> + if (phy->attrs.mode == PHY_MODE_LVDS && dphy->variant->is_combo_dphy) {
> + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG1, 0);
> + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG0, 0);
> + }
> +
> return 0;
> }
>
> @@ -533,6 +591,7 @@ static const struct phy_ops sun6i_dphy_ops = {
> .configure = sun6i_dphy_configure,
> .power_on = sun6i_dphy_power_on,
> .power_off = sun6i_dphy_power_off,
> + .set_mode = sun6i_dphy_set_mode,
> .init = sun6i_dphy_init,
> .exit = sun6i_dphy_exit,
> };
> @@ -619,6 +678,8 @@ static const struct sun6i_dphy_variant sun6i_a31_mipi_dphy_variant = {
>
> static const struct sun6i_dphy_variant sun50i_a100_mipi_dphy_variant = {
> .tx_power_on = sun50i_a100_mipi_dphy_tx_power_on,
> + .lvds_power_on = sun50i_a100_mipi_dphy_lvds_power_on,
> + .is_combo_dphy = true,
> };
>
> static const struct of_device_id sun6i_dphy_of_table[] = {
> --
> 2.25.1
>
>
--
Paul Kocialkowski,
Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/
Expert in multimedia, graphics and embedded hardware support with Linux.
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists