From: Anson Huang <[email protected]>
i.MX8MM SoC has a subset of i.MX8MQ IP block variant, it can reuse
the i.MX8MQ reset controller driver and just skip those non-existing
IP blocks, add support for i.MX8MM SoC reset control.
Signed-off-by: Anson Huang <[email protected]>
---
drivers/reset/reset-imx7.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
index 3ecd770..941131f 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -207,6 +207,26 @@ static int imx8mq_reset_set(struct reset_controller_dev *rcdev,
const unsigned int bit = imx7src->signals[id].bit;
unsigned int value = assert ? bit : 0;
+ if (of_machine_is_compatible("fsl,imx8mm")) {
+ switch (id) {
+ case IMX8MQ_RESET_HDMI_PHY_APB_RESET:
+ case IMX8MQ_RESET_PCIEPHY2: /* fallthrough */
+ case IMX8MQ_RESET_PCIEPHY2_PERST: /* fallthrough */
+ case IMX8MQ_RESET_PCIE2_CTRL_APPS_EN: /* fallthrough */
+ case IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF: /* fallthrough */
+ case IMX8MQ_RESET_MIPI_CSI1_CORE_RESET: /* fallthrough */
+ case IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET: /* fallthrough */
+ case IMX8MQ_RESET_MIPI_CSI1_ESC_RESET: /* fallthrough */
+ case IMX8MQ_RESET_MIPI_CSI2_CORE_RESET: /* fallthrough */
+ case IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET: /* fallthrough */
+ case IMX8MQ_RESET_MIPI_CSI2_ESC_RESET: /* fallthrough */
+ case IMX8MQ_RESET_DDRC2_PHY_RESET: /* fallthrough */
+ case IMX8MQ_RESET_DDRC2_CORE_RESET: /* fallthrough */
+ case IMX8MQ_RESET_DDRC2_PRST: /* fallthrough */
+ return 0;
+ }
+ }
+
switch (id) {
case IMX8MQ_RESET_PCIEPHY:
case IMX8MQ_RESET_PCIEPHY2: /* fallthrough */
--
2.7.4
From: Anson Huang <[email protected]>
i.MX8MM can reuse i.MX8MQ's src driver, add "fsl,imx8mq-src" as
src's fallback compatible to enable it.
Signed-off-by: Anson Huang <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index f0ac027..ea15457 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -471,7 +471,7 @@
};
src: reset-controller@30390000 {
- compatible = "fsl,imx8mm-src", "syscon";
+ compatible = "fsl,imx8mm-src", "fsl,imx8mq-src", "syscon";
reg = <0x30390000 0x10000>;
interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
#reset-cells = <1>;
--
2.7.4
Hi Anson,
On Mon, 2019-07-01 at 17:39 +0800, [email protected] wrote:
> From: Anson Huang <[email protected]>
>
> i.MX8MM SoC has a subset of i.MX8MQ IP block variant, it can reuse
> the i.MX8MQ reset controller driver and just skip those non-existing
> IP blocks, add support for i.MX8MM SoC reset control.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> drivers/reset/reset-imx7.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 3ecd770..941131f 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -207,6 +207,26 @@ static int imx8mq_reset_set(struct reset_controller_dev *rcdev,
> const unsigned int bit = imx7src->signals[id].bit;
> unsigned int value = assert ? bit : 0;
>
> + if (of_machine_is_compatible("fsl,imx8mm")) {
This should be checked once during probe, not in every reset_set, if
this check has to be made at all. On i.MX8MM these unused reset controls
are not going to be hooked up via phandle, so this function should never
be called with the values that are filtered out here anyway.
And in case somebody makes an error in the device tree, does writing 1
to the reserved bits on i.MX8MM have any negative side effects at all?
Or are these bits just not hooked up? If this is no problem, I assume
this patch is not needed at all.
The correct way to protect against DT writers hooking up the non-
existing reset lines would be to replace rcdev.of_xlate with a version
that returns -EINVAL for them on i.MX8MM. Also in that case I'd check
the reset-controller compatible, not the machine compatible.
> + switch (id) {
> + case IMX8MQ_RESET_HDMI_PHY_APB_RESET:
> + case IMX8MQ_RESET_PCIEPHY2: /* fallthrough */
> + case IMX8MQ_RESET_PCIEPHY2_PERST: /* fallthrough */
> + case IMX8MQ_RESET_PCIE2_CTRL_APPS_EN: /* fallthrough */
> + case IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF: /* fallthrough */
> + case IMX8MQ_RESET_MIPI_CSI1_CORE_RESET: /* fallthrough */
> + case IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET: /* fallthrough */
> + case IMX8MQ_RESET_MIPI_CSI1_ESC_RESET: /* fallthrough */
> + case IMX8MQ_RESET_MIPI_CSI2_CORE_RESET: /* fallthrough */
> + case IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET: /* fallthrough */
> + case IMX8MQ_RESET_MIPI_CSI2_ESC_RESET: /* fallthrough */
> + case IMX8MQ_RESET_DDRC2_PHY_RESET: /* fallthrough */
> + case IMX8MQ_RESET_DDRC2_CORE_RESET: /* fallthrough */
> + case IMX8MQ_RESET_DDRC2_PRST: /* fallthrough */
I think it would make sense to add IMX8MM_RESET_* defines for all but
these, to avoid confusion when reading the imx8mm.dtsi
regards
Philipp
Hi, Philipp
> On Mon, 2019-07-01 at 17:39 +0800, [email protected] wrote:
> > From: Anson Huang <[email protected]>
> >
> > i.MX8MM SoC has a subset of i.MX8MQ IP block variant, it can reuse the
> > i.MX8MQ reset controller driver and just skip those non-existing IP
> > blocks, add support for i.MX8MM SoC reset control.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > drivers/reset/reset-imx7.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> > index 3ecd770..941131f 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -207,6 +207,26 @@ static int imx8mq_reset_set(struct
> reset_controller_dev *rcdev,
> > const unsigned int bit = imx7src->signals[id].bit;
> > unsigned int value = assert ? bit : 0;
> >
> > + if (of_machine_is_compatible("fsl,imx8mm")) {
>
> This should be checked once during probe, not in every reset_set, if this
> check has to be made at all. On i.MX8MM these unused reset controls are
> not going to be hooked up via phandle, so this function should never be
> called with the values that are filtered out here anyway.
> And in case somebody makes an error in the device tree, does writing 1 to
> the reserved bits on i.MX8MM have any negative side effects at all?
> Or are these bits just not hooked up? If this is no problem, I assume this
> patch is not needed at all.
I just tried it on i.MX8MM, read/write to the reserved registers looks like OK,
so this patch can be ignored, although accessing reserved registers is NOT good,
but the user should have correct settings in DT, there will be no access for these
reserved registers.
So, let's just ignore this patch, adding the fallback compatible should be good for
i.MX8MM to reuse this driver.
Thanks,
Anson
>
> The correct way to protect against DT writers hooking up the non- existing
> reset lines would be to replace rcdev.of_xlate with a version that returns -
> EINVAL for them on i.MX8MM. Also in that case I'd check the reset-controller
> compatible, not the machine compatible.
>
> > + switch (id) {
> > + case IMX8MQ_RESET_HDMI_PHY_APB_RESET:
> > + case IMX8MQ_RESET_PCIEPHY2: /* fallthrough */
> > + case IMX8MQ_RESET_PCIEPHY2_PERST: /* fallthrough */
> > + case IMX8MQ_RESET_PCIE2_CTRL_APPS_EN: /* fallthrough
> */
> > + case IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF: /*
> fallthrough */
> > + case IMX8MQ_RESET_MIPI_CSI1_CORE_RESET: /* fallthrough
> */
> > + case IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET: /*
> fallthrough */
> > + case IMX8MQ_RESET_MIPI_CSI1_ESC_RESET: /* fallthrough
> */
> > + case IMX8MQ_RESET_MIPI_CSI2_CORE_RESET: /* fallthrough
> */
> > + case IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET: /*
> fallthrough */
> > + case IMX8MQ_RESET_MIPI_CSI2_ESC_RESET: /* fallthrough
> */
> > + case IMX8MQ_RESET_DDRC2_PHY_RESET: /* fallthrough */
> > + case IMX8MQ_RESET_DDRC2_CORE_RESET: /* fallthrough */
> > + case IMX8MQ_RESET_DDRC2_PRST: /* fallthrough */
>
> I think it would make sense to add IMX8MM_RESET_* defines for all but
> these, to avoid confusion when reading the imx8mm.dtsi
>
> regards
> Philipp
Hi Anson,
On Mon, 2019-07-01 at 17:39 +0800, [email protected] wrote:
> From: Anson Huang <[email protected]>
>
> i.MX8MM can reuse i.MX8MQ's src driver, add "fsl,imx8mq-src" as
> src's fallback compatible to enable it.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index f0ac027..ea15457 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -471,7 +471,7 @@
> };
>
> src: reset-controller@30390000 {
> - compatible = "fsl,imx8mm-src", "syscon";
> + compatible = "fsl,imx8mm-src", "fsl,imx8mq-src", "syscon";
> reg = <0x30390000 0x10000>;
> interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> #reset-cells = <1>;
Please also update the compatible property documentation in
Documentation/devicetree/bindings/reset/fsl,imx7-src.txt.
With that,
Reviewed-by: Philipp Zabel <[email protected]>
regards
Philipp