Re: [RFC] Configure i.MX6 RGMII pad group control registers from device tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25.6.2018 04:50, Andy Duan wrote:
On 11.6.2018 14:36, Michal Vokáč wrote:
Ahoj,

To configure individual pad's characteristics on i.MX6 SoC a
fsl,pins = <PIN_FUNC_ID CONFIG> property can be used. Is there any
convenient way to configure the pad group control registers?

The issue is that some bits (DDR_SEL and ODT) in the individual
RGMII pad control registers are read-only. To tweak those parameters
(signal voltage and termination resistors) one need to write to the
pad group control registers for the whole RGMII pad group. Namely
IOMUXC_SW_PAD_CTL_GRP_DDR_TYPE_RGMII and
IOMUXC_SW_PAD_CTL_GRP_RGMII_TERM. The group registers in general
are not accessible from the list in arch/arm/boot/dts/imx6dl-pinfunc.h.

I could not find any other way to change the group registers than
hacking-in some lines into the imx6q_init_machine(void) function in
arch/arm/mach-imx/mach-imx6q.c source. As I work towards upstreaming
my board this should be done from my device tree or solved in some
universal way.

Any hints will be much appreciated.
Michal

I figured out this is more "pinctrl-imx.c" than "device-tree" related
so I am kindly adding maintainers of that file in hope somebody will
shed some light to it.

I am diving deeper into the code and it seems there really is no
generic option to set the i.MX6 pad group control registers from device
tree. Or am I looking at the problem from a wrong angle?

Yes, there's a few special pad group ctrl registers (e.g. DRAM and RGMII
for mx6q) which are not added In the pinctrl driver support.

Hi Andy and Dong! Thank you very much for your comments.

I still have quite limited knowledge about the pinctrl driver and related
things but AFAIK it is not like that few pad group ctrl registers are not
supported. I do not see any support for group control registers at all.
And not only for the imx6q but for all the SoC variants and other SoCs as
well. Am I right?

How should we deal with boards that need to configure some pad
characteristics available only through the pad group control registers?

Andy,
How do we handle it internally?
No, we don't handle the pin.
I remembered IC owner said It seems only RGMII 2.5v need to handle the pin.

That is our case. I need to use 2.5V signaling at the RGMII for
the connected QCA8334 switch. And also to set the terminators accordingly.

There're probably two ways to do it:
1) handle it in fec driver by parsing a specific property
2) Add a new pad group into pinctrl driver support e.g.
MX6Q_PAD_CTL_GRP_RGMII_TERM
MX6Q_PAD_CTL_GRP_DDR_TYPE_RGMII

I may prefer to 2).

No.1 is similar to what I am doing now. I have a DT node with custom
compatible string and a reg property. Then I look for that compatible from
imx6q_enet_init() using syscon_regmap_lookup_by_compatible("fsl,imx6-rgmii-ddrtype-gpr");
I do not see a chance that something like this could be accepted upstream.

No.2 is much more complex. IMHO it is not about adding support for a new
pad group. It is about adding support for pad group control registers from
scratch.

I do not mind working on a proper solution. Though as I mentioned I am
still not very experienced in kernel internals/APIs so I will appreciate
some guidance along the way. It should not be as complex as
handling pin muxing and all the related things though.

What I see as a potential problem is conflict of the usage of the "pin group"
term. Now "pin group" is used in pinctrl core and refers to a DT node.
That node associates any pins that are needed for a given functionality.
Those pins can actually be wired to totally different IP blocks of the SoC.

Whereas the pad group control register typically associates and controls
pins that are common to one IP block.

So the question is how complex such implementation should be?
How should the binding look like?
What is the proper place to parse the DT and write the registers?
What SoCs should be supported?

Se bellow my very preliminary proposal how this may look like.
It is meant more like a background for further discussion.
I am really not sure if this should be strictly solved at the imx-pinctrl
level or if this overlaps into the pinctrl core.

Thanks a lot for any additional comments,
Michal

diff --git a/arch/arm/boot/dts/imx6dl-pinfunc.h b/arch/arm/boot/dts/imx6dl-pinfunc.h
index 37e430a..eeac9e3 100644
--- a/arch/arm/boot/dts/imx6dl-pinfunc.h
+++ b/arch/arm/boot/dts/imx6dl-pinfunc.h
@@ -1089,4 +1089,24 @@
 #define MX6QDL_PAD_SD4_DAT7__UART2_RX_DATA          0x35c 0x744 0x904 0x2 0x7
 #define MX6QDL_PAD_SD4_DAT7__GPIO2_IO15             0x35c 0x744 0x000 0x5 0x0
+/* Pad group control registers */
+#define MX6QDL_PAD_CTL_GRP_B7DS           0x748
+#define MX6QDL_PAD_CTL_GRP_ADDDS          0x74c
+#define MX6QDL_PAD_CTL_GRP_DDRMODE_CTL    0x750
+#define MX6QDL_PAD_CTL_GRP_DDRPKE         0x754
+#define MX6QDL_PAD_CTL_GRP_DDRPK          0x758
+#define MX6QDL_PAD_CTL_GRP_DDRHYS         0x75c
+#define MX6QDL_PAD_CTL_GRP_DDRMODE        0x760
+#define MX6QDL_PAD_CTL_GRP_B0DS           0x764
+#define MX6QDL_PAD_CTL_GRP_DDR_TYPE_RGMII 0x768
+#define MX6QDL_PAD_CTL_GRP_CTLDS          0x76c
+#define MX6QDL_PAD_CTL_GRP_B1DS           0x770
+#define MX6QDL_PAD_CTL_GRP_DDR_TYPE       0x774
+#define MX6QDL_PAD_CTL_GRP_B2DS           0x778
+#define MX6QDL_PAD_CTL_GRP_B3DS           0x77c
+#define MX6QDL_PAD_CTL_GRP_B4DS           0x780
+#define MX6QDL_PAD_CTL_GRP_B5DS           0x784
+#define MX6QDL_PAD_CTL_GRP_RGMII_TERM     0x788
+#define MX6QDL_PAD_CTL_GRP_B6DS           0x78c
+
 #endif /* __DTS_IMX6DL_PINFUNC_H */
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index 15744ad..3e9d1ba 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -467,6 +467,11 @@
 				MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL	0x1b030
 				MX6QDL_PAD_GPIO_16__ENET_REF_CLK	0x4001b0a8
 			>;
+
+			fsl,pin-groups = <
+				MX6QDL_PAD_CTL_GRP_RGMII_TERM           0xC0000
+				MX6QDL_PAD_CTL_GRP_DDR_TYPE_RGMII       0x100
+			>;
 		};
pinctrl_gpio_keys: gpio_keysgrp {
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 1c6bb15..3c42917 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -235,6 +235,8 @@ static int imx_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 		}
 	}
+ /* TODO write the pad group control registers here? */
+
 	return 0;
 }
@@ -421,6 +423,7 @@ static const struct pinconf_ops imx_pinconf_ops = {
  *     <mux_conf_reg input_reg mux_mode input_val>
  */
 #define FSL_PIN_SIZE 24
+#define FSL_PIN_GRP_SIZE 8
 #define FSL_PIN_SHARE_SIZE 20
static int imx_pinctrl_parse_groups(struct device_node *np,
@@ -430,6 +433,7 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
 {
 	const struct imx_pinctrl_soc_info *info = ipctl->info;
 	int size, pin_size;
+	int pin_grp_size = FSL_PIN_GRP_SIZE;
 	const __be32 *list;
 	int i;
 	u32 config;
@@ -531,6 +539,22 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
 				pin->mux_mode, pin->config);
 	}
+ /* parse the pad control group register configuration */
+	list = of_get_property(np, "fsl,pin-groups", &size);
+
+	/* this binding is optional so stop here if it is not used */
+	if (!list)
+		goto out;
+
+	/* we do not check return since it's safe node passed down */
+	if (!size || size % pin_grp_size) {
+		dev_err(ipctl->dev, "Invalid fsl,pin-groups property in node %pOF\n", np);
+		return -EINVAL;
+	}
+
+	/* TODO Parse the pad group register IDs and its configuration values */
+
+out:
 	return 0;
 }
--
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux