Re: [PATCH v3 2/2] phy: amlogic: Add G12A Analog MIPI D-PHY driver
From: Neil Armstrong <hidden>
Date: 2022-01-10 13:05:00
Also in:
linux-amlogic, linux-arm-kernel, lkml
Hi, On 07/01/2022 23:09, Martin Blumenstingl wrote:
Hi Neil, On Fri, Jan 7, 2022 at 4:06 PM Neil Armstrong [off-list ref] wrote: [...]quoted
+#define HHI_MIPI_CNTL2 0x08 +#define HHI_MIPI_CNTL2_DIF_TX_CTL1 GENMASK(31, 16) +#define HHI_MIPI_CNTL2_CH_EN GENMASK(15, 11) +#define HHI_MIPI_CNTL2_DIF_TX_CTL0 GENMASK(10, 0) + +#define DSI_LANE_0 BIT(4) +#define DSI_LANE_1 BIT(3) +#define DSI_LANE_CLK BIT(2) +#define DSI_LANE_2 BIT(1) +#define DSI_LANE_3 BIT(0)At first I thought that these should be named HHI_MIPI_CNTL2_DSI_LANE_0 (and similar). But then I understood that they aren't bits directly in HHI_MIPI_CNTL2 but they belong to HHI_MIPI_CNTL2_CH_EN. Have you considered naming them for example HHI_MIPI_CNTL2_CH_EN_DSI_LANE_0 to make this more clear? [...]quoted
+ if (IS_ERR(map)) { + dev_err(dev, + "failed to get HHI regmap\n"); + return PTR_ERR(map);I suggest using: return dev_err_probe(dev, PTR_ERR(map), "failed to get HHI regmap\n"); to simplify the code [...]quoted
+ if (IS_ERR(priv->phy)) { + ret = PTR_ERR(priv->phy); + if (ret != -EPROBE_DEFER) + dev_err(dev, "failed to create PHY\n"); + return ret;and similar here: return dev_err_probe(dev, PTR_ERR(priv->phy), "failed to create PHY\n"); [...]quoted
+static const struct of_device_id phy_g12a_mipi_dphy_analog_of_match[] = { + { + .compatible = "amlogic,g12a-mipi-dphy-analog", + }, + { },In the past I was suggested to use: { /* sentinel */ } meaning: no trailing comma here so nobody can add entries after the sentinel by accident. I suggest doing the same here if you re-spin this series.
Yep, will do the changes, Thanks, Neil
Thank you! Martin
-- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy