Hi Vinod,
Am Freitag, dem 13.01.2023 um 23:24 +0530 schrieb Vinod Koul:
> On 15-12-22, 21:11, Lucas Stach wrote:
> > This adds the driver for the Samsung HDMI PHY found on the
> > i.MX8MP SoC. Based on downstream implementation from
> > Sandor Yu <Sandor.yu@xxxxxxx>.
> >
> > Co-developed-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> > Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > ---
> > v2: use DEFINE_RUNTIME_DEV_PM_OPS
> > ---
> > drivers/phy/freescale/Kconfig | 6 +
> > drivers/phy/freescale/Makefile | 1 +
> > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 694 +++++++++++++++++++
> > 3 files changed, 701 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
>
> this new should come after this one...
>
> > diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
> > index cedb328bc4d2..c4386bfdb853 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..185244dcb810
> > --- /dev/null
> > +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> > @@ -0,0 +1,694 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2020 NXP
> > + * Copyright 2022 Pengutronix, Lucas Stach <kernel@xxxxxxxxxxxxxx>
> > + */
> > +
> > +#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_00 0x00
> > +#define PHY_REG_01 0x04
> > +#define PHY_REG_02 0x08
> > +#define PHY_REG_08 0x20
> > +#define PHY_REG_09 0x24
> > +#define PHY_REG_10 0x28
> > +#define PHY_REG_11 0x2c
>
> No names for regs?
Unfortunately yes. While the reference manual documents the bitfields
in the regs, it doesn't give them any descriptive names, but just those
consecutive numbers. I don't think it would be a good idea to deviate
from the reference manual here.
>
> > +
> > +#define PHY_REG_12 0x30
> > +#define REG12_FLD_CK_DIV(x) (((x) & 0x3) << 4)
>
> GENMASK() pls
Okay.
>
> > +#define REG12_TMDS_CLK 0x0
> > +#define REG12_TMDS_CLK_DIV2 0x1
> > +#define REG12_TMDS_CLK_DIV4 0x2
> > +#define REG12_TMDS_CLK_DIV8 0x3
> > +
> > +#define PHY_REG_13 0x34
> > +#define REG13_FLD_TG_CODE_LOW(x) (x & 0xff)
> > +
> > +#define PHY_REG_14 0x38
> > +#define REG14_FLD_TOL(x) ((x & 0xf) << 4)
> > +#define REG14_FLD_RP_CODE(x) ((x & 0x3) << 1)
> > +#define REG14_FLD_TG_CODE_HIGH(x) ((x >> 8) & 0x1)
>
> FIELD_GET|PREP please
Okay.
>
> > +
> > +#define PHY_REG_15 0x3c
> > +#define PHY_REG_16 0x40
> > +#define PHY_REG_17 0x44
> > +#define PHY_REG_18 0x48
> > +#define PHY_REG_19 0x4c
> > +#define PHY_REG_20 0x50
> > +
> > +#define PHY_REG_21 0x54
> > +#define REG21_SEL_TX_CK_INV BIT(7)
> > +#define REG21_PMS_S(x) (x & 0xf)
> > +
> > +#define PHY_REG_22 0x58
> > +#define PHY_REG_23 0x5c
> > +#define PHY_REG_24 0x60
> > +#define PHY_REG_25 0x64
> > +#define PHY_REG_26 0x68
> > +#define PHY_REG_27 0x6c
> > +#define PHY_REG_28 0x70
> > +#define PHY_REG_29 0x74
> > +#define PHY_REG_30 0x78
> > +#define PHY_REG_31 0x7c
> > +#define PHY_REG_32 0x80
> > +
> > +#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_REG_35 0x8c
> > +#define PHY_REG_36 0x90
> > +#define PHY_REG_37 0x94
> > +#define PHY_REG_38 0x98
> > +#define PHY_REG_39 0x9c
> > +#define PHY_REG_40 0xa0
> > +#define PHY_REG_41 0xa4
> > +#define PHY_REG_42 0xa8
> > +#define PHY_REG_43 0xac
> > +#define PHY_REG_44 0xb0
> > +#define PHY_REG_45 0xb4
> > +#define PHY_REG_46 0xb8
> > +#define PHY_REG_47 0xbc
> > +
> > +#define PHY_PLL_DIV_REGS_NUM 6
> > +
> > +struct phy_config {
> > + u32 pixclk;
> > + u8 pll_div_regs[PHY_PLL_DIV_REGS_NUM];
> > +};
> > +
> > +const struct phy_config phy_pll_cfg[] = {
> > + {
> > + .pixclk = 22250000,
> > + .pll_div_regs = { 0x4B, 0xF1, 0x89, 0x88, 0x80, 0x40 },
>
> small case for hex numbers pls
Ack.
>
> > + }, {
> > + .pixclk = 23750000,
> > + .pll_div_regs = { 0x50, 0xF1, 0x86, 0x85, 0x80, 0x40 },
> > + },{
> >
[...]
> > + .pixclk = 288000000,
> > + .pll_div_regs = { 0x78, 0x10, 0x00, 0x00, 0x80, 0x00 },
> > + }, {
> > + .pixclk = 297000000,
> > + .pll_div_regs = { 0x7B, 0x15, 0x84, 0x03, 0x90, 0x45 },
> > + },
>
> lots of magic numbers!
Yes. Those are mostly tuning values for the PLL and I don't know if
there is any computational way to come up with those numbers, so we're
just using the values validated in downstream.
>
> > +};
> > +
> > +struct reg_settings {
> > + u8 reg;
> > + u8 val;
> > +};
> > +
> > +const struct reg_settings common_phy_cfg[] = {
> > + { PHY_REG_00, 0x00 }, { PHY_REG_01, 0xD1 },
> > + { PHY_REG_08, 0x4f }, { PHY_REG_09, 0x30 },
> > + { PHY_REG_10, 0x33 }, { PHY_REG_11, 0x65 },
> > + /* REG12 pixclk specific */
> > + /* REG13 pixclk specific */
> > + /* REG14 pixclk specific */
> > + { PHY_REG_15, 0x80 }, { PHY_REG_16, 0x6C },
> > + { PHY_REG_17, 0xF2 }, { PHY_REG_18, 0x67 },
> > + { PHY_REG_19, 0x00 }, { PHY_REG_20, 0x10 },
> > + /* REG21 pixclk specific */
> > + { PHY_REG_22, 0x30 }, { PHY_REG_23, 0x32 },
> > + { PHY_REG_24, 0x60 }, { PHY_REG_25, 0x8F },
> > + { PHY_REG_26, 0x00 }, { PHY_REG_27, 0x00 },
> > + { PHY_REG_28, 0x08 }, { PHY_REG_29, 0x00 },
> > + { PHY_REG_30, 0x00 }, { PHY_REG_31, 0x00 },
> > + { PHY_REG_32, 0x00 }, { PHY_REG_33, 0x80 },
> > + { PHY_REG_34, 0x00 }, { PHY_REG_35, 0x00 },
> > + { PHY_REG_36, 0x00 }, { PHY_REG_37, 0x00 },
> > + { PHY_REG_38, 0x00 }, { PHY_REG_39, 0x00 },
> > + { PHY_REG_40, 0x00 }, { PHY_REG_41, 0xE0 },
> > + { PHY_REG_42, 0x83 }, { PHY_REG_43, 0x0F },
> > + { PHY_REG_44, 0x3E }, { PHY_REG_45, 0xF8 },
> > + { PHY_REG_46, 0x00 }, { PHY_REG_47, 0x00 }
> > +};
> > +
> > +struct fsl_samsung_hdmi_phy {
> > + struct device *dev;
> > + void __iomem *regs;
> > + struct clk *apbclk;
> > + struct clk *refclk;
> > +
> > + /* clk provider */
> > + struct clk_hw hw;
> > + const struct phy_config *cur_cfg;
> > +};
> > +
> > +static inline struct fsl_samsung_hdmi_phy *
> > +to_fsl_samsung_hdmi_phy(struct clk_hw *hw)
> > +{
> > + return container_of(hw, struct fsl_samsung_hdmi_phy, hw);
> > +}
> > +
> > +static void
> > +fsl_samsung_hdmi_phy_configure_pixclk(struct fsl_samsung_hdmi_phy *phy,
> > + const struct phy_config *cfg)
> > +{
> > + u8 div;
> > +
> > + switch (cfg->pixclk) {
> > + case 22250000 ... 33750000: div = 0xf; break;
> > + case 35000000 ... 40000000: div = 0xb; break;
> > + case 43200000 ... 47500000: div = 0x9; break;
> > + case 50349650 ... 63500000: div = 0x7; break;
> > + case 67500000 ... 90000000: div = 0x5; break;
> > + case 94000000 ... 148500000: div = 0x3; break;
> > + case 154000000 ... 297000000: div = 0x1; break;
>
> lets do proper linux style please
Do you mean moving the statements to separate lines?
Regards,
Lucas
