[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240205081719.z2uqa4dwn5ucsymv@pengutronix.de>
Date: Mon, 5 Feb 2024 09:17:19 +0100
From: Marco Felsch <m.felsch@...gutronix.de>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Adam Ford <aford173@...il.com>, Ulf Hansson <ulf.hansson@...aro.org>,
Andrzej Hajda <andrzej.hajda@...el.com>, devicetree@...r.kernel.org,
alexander.stein@...tq-group.com,
Catalin Marinas <catalin.marinas@....com>,
dri-devel@...ts.freedesktop.org, frieder.schrempf@...tron.de,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-phy@...ts.infradead.org, David Airlie <airlied@...il.com>,
marex@...x.de, Robert Foss <rfoss@...nel.org>,
Fabio Estevam <festevam@...il.com>, linux-pm@...r.kernel.org,
Jernej Skrabec <jernej.skrabec@...il.com>,
NXP Linux Team <linux-imx@....com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Will Deacon <will@...nel.org>, Jonas Karlman <jonas@...boo.se>,
Liu Ying <victor.liu@....com>,
Sascha Hauer <s.hauer@...gutronix.de>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
linux-arm-kernel@...ts.infradead.org,
Neil Armstrong <neil.armstrong@...aro.org>,
linux-kernel@...r.kernel.org, Vinod Koul <vkoul@...nel.org>,
Daniel Vetter <daniel@...ll.ch>,
Lucas Stach <l.stach@...gutronix.de>,
Shawn Guo <shawnguo@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH V8 02/12] phy: freescale: add Samsung HDMI PHY
On 24-02-04, Dmitry Baryshkov wrote:
> On Sat, 3 Feb 2024 at 17:53, Adam Ford <aford173@...il.com> wrote:
> >
> > From: Lucas Stach <l.stach@...gutronix.de>
> >
> > This adds the driver for the Samsung HDMI PHY found on the
> > i.MX8MP SoC.
> >
> > Signed-off-by: Lucas Stach <l.stach@...gutronix.de>
> > Signed-off-by: Adam Ford <aford173@...il.com>
> > Tested-by: Alexander Stein <alexander.stein@...tq-group.com>
> > ---
> > V4: Make lookup table hex values lower case.
> >
> > V3: Re-order the Makefile to keep it alphabetical
> > Remove unused defines
> >
> > V2: Fixed some whitespace found from checkpatch
> > Change error handling when enabling apbclk to use dev_err_probe
> > Rebase on Linux-Next
> >
> > I (Adam) tried to help move this along, so I took Lucas' patch and
> > attempted to apply fixes based on feedback. I don't have
> > all the history, so apologies for that.
> > ---
> > drivers/phy/freescale/Kconfig | 6 +
> > drivers/phy/freescale/Makefile | 1 +
> > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 ++++++++++++++++++
> > 3 files changed, 1082 insertions(+)
> > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> >
> > diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
> > index 853958fb2c06..5c2b73042dfc 100644
> > --- a/drivers/phy/freescale/Kconfig
> > +++ b/drivers/phy/freescale/Kconfig
> > @@ -35,6 +35,12 @@ config PHY_FSL_IMX8M_PCIE
> > Enable this to add support for the PCIE PHY as found on
> > i.MX8M family of SOCs.
> >
> > +config PHY_FSL_SAMSUNG_HDMI_PHY
> > + tristate "Samsung HDMI PHY support"
> > + depends on OF && HAS_IOMEM
> > + help
> > + Enable this to add support for the Samsung HDMI PHY in i.MX8MP.
> > +
> > endif
> >
> > config PHY_FSL_LYNX_28G
> > diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
> > index cedb328bc4d2..79e5f16d3ce8 100644
> > --- a/drivers/phy/freescale/Makefile
> > +++ b/drivers/phy/freescale/Makefile
> > @@ -4,3 +4,4 @@ obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o
> > obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o
> > obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o
> > obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o
> > +obj-$(CONFIG_PHY_FSL_SAMSUNG_HDMI_PHY) += phy-fsl-samsung-hdmi.o
> > diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> > new file mode 100644
> > index 000000000000..bf0e2299d00f
> > --- /dev/null
> > +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> > @@ -0,0 +1,1075 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2020 NXP
> > + * Copyright 2022 Pengutronix, Lucas Stach <kernel@...gutronix.de>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#define PHY_REG_33 0x84
> > +#define REG33_MODE_SET_DONE BIT(7)
> > +#define REG33_FIX_DA BIT(1)
> > +
> > +#define PHY_REG_34 0x88
> > +#define REG34_PHY_READY BIT(7)
> > +#define REG34_PLL_LOCK BIT(6)
> > +#define REG34_PHY_CLK_READY BIT(5)
> > +
> > +
> > +#define PHY_PLL_REGS_NUM 48
> > +
> > +struct phy_config {
> > + u32 clk_rate;
> > + u8 regs[PHY_PLL_REGS_NUM];
> > +};
> > +
> > +const struct phy_config phy_pll_cfg[] = {
> > + { 22250000, {
> > + 0x00, 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40,
> > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x15, 0x25, 0x80,
> > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32,
> > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
> > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00,
> > + },
> > + }, {
> > + 23750000, {
> > + 0x00, 0xd1, 0x50, 0xf1, 0x86, 0x85, 0x80, 0x40,
> > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x03, 0x25, 0x80,
> > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32,
> > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
> > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00,
> > + },
>
> Generally I see that these entries contain a high level of duplication.
> Could you please extract the common part and a rate-dependent part.
> Next, it would be best if instead of writing the register values via
> the rate LUT, the driver could calculate those values.
> This allows us to support other HDMI modes if the need arises at some point.
Hi Adam,
can you please have a look at: https://lore.kernel.org/all/4830698.GXAFRqVoOG@steina-w/
there we have fixed this already. Not sure which version you picked for
your work.
Regards,
Marco
>
> > + }, {
> > + 24000000, {
> > + 0x00, 0xd1, 0x50, 0xf0, 0x00, 0x00, 0x80, 0x00,
> > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x01, 0x25, 0x80,
> > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32,
> > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
> > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00,
> > + },
>
>
> --
> With best wishes
> Dmitry
>
>
Powered by blists - more mailing lists