RE: [PATCH V6 0/2] can: flexcan: add can self wakeup support
> -Original Message- > From: Joakim Zhang > Sent: Friday, November 23, 2018 4:35 PM > To: linux-...@vger.kernel.org; m...@pengutronix.de; robh...@kernel.org > Cc: w...@grandegger.com; mark.rutl...@arm.com; devicet...@vger.kernel.org; > linux-kernel@vger.kernel.org; dl-linux-imx ; Joakim > Zhang > Subject: [PATCH V6 0/2] can: flexcan: add can self wakeup support > > This patch series intends to add CAN self wakeup support. The CAN controller > can parse "fsl,stop-mode" property from device tree to enable self wakeup > feature. > As I'm already authored, this is just to tell this version looks good to me. Reviewed-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH V3 1/1] can: flexcan: Add self wakeup support
> -Original Message-
> From: Joakim Zhang
[...]
>
> If wakeup is enabled, enter stop mode, else enter disabled mode.Self wake can
> only work on stop mode.
>
> Starting from IMX6, the flexcan stop mode control bits is SoC specific, move
> it
> out of IP driver and parse it from devicetree.
>
> Signed-off-by: Dong Aisheng
> Signed-off-by: Joakim Zhang
> ---
> drivers/net/can/flexcan.c | 175
> +++---
> 1 file changed, 163 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index
> 3813f6708201..d21d0db8c83e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -19,12 +19,17 @@
> #include
> #include
> #include
> +#include
> +#include
This two lines seems not neccesary
> +#include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> #define DRV_NAME "flexcan"
>
> @@ -132,7 +137,8 @@
> (FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE) #define
> FLEXCAN_ESR_ALL_INT \
> (FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> - FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> + FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> + FLEXCAN_ESR_WAK_INT)
>
> /* FLEXCAN interrupt flag register (IFLAG) bits */
> /* Errata ERR005829 step7: Reserve first valid MB */ @@ -191,6 +197,7 @@
> #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP BIT(5) /* Use timestamp
> based offloading */
> #define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6) /* No interrupt for
> error passive */
> #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7) /* default to BE
> register access */
> +#define FLEXCAN_QUIRK_SETUP_STOP_MODEBIT(8) /* Setup stop mode to
> support wakeup */
>
> /* Structure of the message buffer */
> struct flexcan_mb {
> @@ -255,6 +262,14 @@ struct flexcan_devtype_data {
> u32 quirks; /* quirks needed for different IP cores */
> };
>
> +struct flexcan_stop_mode {
> + struct regmap *gpr;
> + u8 req_gpr;
> + u8 req_bit;
> + u8 ack_gpr;
> + u8 ack_bit;
> +};
> +
> struct flexcan_priv {
> struct can_priv can;
> struct can_rx_offload offload;
> @@ -272,6 +287,7 @@ struct flexcan_priv {
> struct clk *clk_per;
> const struct flexcan_devtype_data *devtype_data;
> struct regulator *reg_xceiver;
> + struct flexcan_stop_mode stm;
>
> /* Read and Write APIs */
> u32 (*read)(void __iomem *addr);
> @@ -295,7 +311,8 @@ static const struct flexcan_devtype_data
> fsl_imx28_devtype_data = {
>
> static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
> .quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> - FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE,
> + FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> + FLEXCAN_QUIRK_SETUP_STOP_MODE,
> };
>
> static const struct flexcan_devtype_data fsl_vf610_devtype_data = { @@
> -392,6 +409,40 @@ static void flexcan_clks_disable(const struct flexcan_priv
> *priv)
> clk_disable_unprepare(priv->clk_per);
> }
>
> +static void flexcan_wake_mask_enable(struct flexcan_priv *priv) {
> + struct flexcan_regs __iomem *regs = priv->regs;
> + u32 reg_mcr;
> +
> + reg_mcr = priv->read(®s->mcr);
> + reg_mcr |= FLEXCAN_MCR_WAK_MSK;
> + priv->write(reg_mcr, ®s->mcr);
> +}
> +
> +static void flexcan_wake_mask_disable(struct flexcan_priv *priv) {
> + struct flexcan_regs __iomem *regs = priv->regs;
> + u32 reg_mcr;
> +
> + reg_mcr = priv->read(®s->mcr);
> + reg_mcr &= ~FLEXCAN_MCR_WAK_MSK;
> + priv->write(reg_mcr, ®s->mcr);
> +}
> +
Please combine those two functions into one, also improve the name a bit.
e.g.
flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
> +static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv) {
> + /* enable stop request */
> + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> + 1 << priv->stm.req_bit, 1 <<
> priv->stm.req_bit); }
> +
> +static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv) {
> + /* remove stop request */
> + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> + 1 << priv->stm.req_bit, 0);
> +}
> +
> static inline int flexcan_transceiver_enable(const struct flexcan_priv
> *priv) {
> if (!priv->reg_xceiver)
> @@ -955,6 +1006,10 @@ static int flexcan_chip_start(struct net_device *dev)
> reg_mcr |= FLEXCAN_MCR_FEN |
> FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
> }
> +
> + /* enable self wakeup */
> + reg_mcr |= FLEXCAN_MCR_SLF_WAK;
> +
Should we check before setting as we already have a SETUP_STOP_MODE quirk?
> netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
>
RE: [PATCH V4 1/1] can: flexcan: add self wakeup support
This mostly looks good to me.
A few minor comments
> -Original Message-
> From: Joakim Zhang
> Sent: Wednesday, November 21, 2018 8:32 PM
> To: linux-...@vger.kernel.org; m...@pengutronix.de
> Cc: w...@grandegger.com; linux-kernel@vger.kernel.org; dl-linux-imx
> ; Aisheng DONG ; Joakim
> Zhang
> Subject: [PATCH V4 1/1] can: flexcan: add self wakeup support
>
> From: Aisheng Dong
>
> If wakeup is enabled, enter stop mode, else enter disabled mode. Self wake can
> only work on stop mode.
>
> Starting from IMX6, the flexcan stop mode control bits is SoC specific, move
> it
> out of IP driver and parse it from devicetree.
>
> Signed-off-by: Aisheng Dong
> Signed-off-by: Joakim Zhang
> ---
> drivers/net/can/flexcan.c | 163
> +++---
> 1 file changed, 154 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index
> 8e972ef08637..83431810316e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -19,11 +19,14 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> #define DRV_NAME "flexcan"
>
> @@ -131,7 +134,8 @@
> (FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE) #define
> FLEXCAN_ESR_ALL_INT \
> (FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> - FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> + FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> + FLEXCAN_ESR_WAK_INT)
>
> /* FLEXCAN interrupt flag register (IFLAG) bits */
> /* Errata ERR005829 step7: Reserve first valid MB */ @@ -190,6 +194,7 @@
> #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP BIT(5) /* Use timestamp
> based offloading */
> #define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6) /* No interrupt for
> error passive */
> #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7) /* default to BE
> register access */
> +#define FLEXCAN_QUIRK_SETUP_STOP_MODEBIT(8) /* Setup stop
> mode to support wakeup */
>
> /* Structure of the message buffer */
> struct flexcan_mb {
> @@ -254,6 +259,14 @@ struct flexcan_devtype_data {
> u32 quirks; /* quirks needed for different IP cores */
> };
>
> +struct flexcan_stop_mode {
> + struct regmap *gpr;
> + u8 req_gpr;
> + u8 req_bit;
> + u8 ack_gpr;
> + u8 ack_bit;
> +};
> +
> struct flexcan_priv {
> struct can_priv can;
> struct can_rx_offload offload;
> @@ -270,6 +283,7 @@ struct flexcan_priv {
> struct clk *clk_per;
> const struct flexcan_devtype_data *devtype_data;
> struct regulator *reg_xceiver;
> + struct flexcan_stop_mode stm;
>
> /* Read and Write APIs */
> u32 (*read)(void __iomem *addr);
> @@ -293,7 +307,8 @@ static const struct flexcan_devtype_data
> fsl_imx28_devtype_data = {
>
> static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
> .quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> - FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE,
> + FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> + FLEXCAN_QUIRK_SETUP_STOP_MODE,
> };
>
> static const struct flexcan_devtype_data fsl_vf610_devtype_data = { @@
> -353,6 +368,35 @@ static inline void flexcan_write_le(u32 val, void __iomem
> *addr)
> iowrite32(val, addr);
> }
>
> +static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool
> +enable) {
> + struct flexcan_regs __iomem *regs = priv->regs;
> + u32 reg_mcr;
> +
> + reg_mcr = priv->read(®s->mcr);
> +
> + if (enable)
> + reg_mcr |= FLEXCAN_MCR_WAK_MSK;
> + else
> + reg_mcr &= ~FLEXCAN_MCR_WAK_MSK;
> +
> + priv->write(reg_mcr, ®s->mcr);
> +}
> +
> +static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv) {
> + /* enable stop request */
> + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +1 << priv->stm.req_bit, 1 << priv->stm.req_bit); }
> +
> +static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv) {
> + /* remove stop request */
> + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +1 << priv->stm.req_bit, 0);
> +}
> +
> static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
> {
> struct flexcan_regs __iomem *regs = priv->regs; @@ -940,6 +984,10 @@
> stat
RE: [PATCH V4 1/1] can: flexcan: add self wakeup support
> -Original Message-
> From: Aisheng DONG
> Sent: Wednesday, November 21, 2018 9:00 PM
> To: Joakim Zhang ; linux-...@vger.kernel.org;
> m...@pengutronix.de
> Cc: w...@grandegger.com; linux-kernel@vger.kernel.org; dl-linux-imx
>
> Subject: RE: [PATCH V4 1/1] can: flexcan: add self wakeup support
>
> This mostly looks good to me.
> A few minor comments
>
BTW, you should re-send the series with binding doc patch if it's still not
merged.
Regards
Dong Aisheng
> > -Original Message-
> > From: Joakim Zhang
> > Sent: Wednesday, November 21, 2018 8:32 PM
> > To: linux-...@vger.kernel.org; m...@pengutronix.de
> > Cc: w...@grandegger.com; linux-kernel@vger.kernel.org; dl-linux-imx
> > ; Aisheng DONG ; Joakim
> Zhang
> >
> > Subject: [PATCH V4 1/1] can: flexcan: add self wakeup support
> >
> > From: Aisheng Dong
> >
> > If wakeup is enabled, enter stop mode, else enter disabled mode. Self
> > wake can only work on stop mode.
> >
> > Starting from IMX6, the flexcan stop mode control bits is SoC
> > specific, move it out of IP driver and parse it from devicetree.
> >
> > Signed-off-by: Aisheng Dong
> > Signed-off-by: Joakim Zhang
> > ---
> > drivers/net/can/flexcan.c | 163
> > +++---
> > 1 file changed, 154 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 8e972ef08637..83431810316e 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -19,11 +19,14 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> > #include
> > #include
> > #include
> > +#include
> >
> > #define DRV_NAME "flexcan"
> >
> > @@ -131,7 +134,8 @@
> > (FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE) #define
> > FLEXCAN_ESR_ALL_INT \
> > (FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> > -FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> > + FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> > + FLEXCAN_ESR_WAK_INT)
> >
> > /* FLEXCAN interrupt flag register (IFLAG) bits */
> > /* Errata ERR005829 step7: Reserve first valid MB */ @@ -190,6 +194,7
> @@
> > #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMPBIT(5) /* Use timestamp
> > based offloading */
> > #define FLEXCAN_QUIRK_BROKEN_PERR_STATEBIT(6) /* No interrupt
> for
> > error passive */
> > #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7) /* default to BE
> > register access */
> > +#define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8) /* Setup stop
> > mode to support wakeup */
> >
> > /* Structure of the message buffer */ struct flexcan_mb { @@ -254,6
> > +259,14 @@ struct flexcan_devtype_data {
> > u32 quirks; /* quirks needed for different IP cores */
> > };
> >
> > +struct flexcan_stop_mode {
> > + struct regmap *gpr;
> > + u8 req_gpr;
> > + u8 req_bit;
> > + u8 ack_gpr;
> > + u8 ack_bit;
> > +};
> > +
> > struct flexcan_priv {
> > struct can_priv can;
> > struct can_rx_offload offload;
> > @@ -270,6 +283,7 @@ struct flexcan_priv {
> > struct clk *clk_per;
> > const struct flexcan_devtype_data *devtype_data;
> > struct regulator *reg_xceiver;
> > + struct flexcan_stop_mode stm;
> >
> > /* Read and Write APIs */
> > u32 (*read)(void __iomem *addr);
> > @@ -293,7 +307,8 @@ static const struct flexcan_devtype_data
> > fsl_imx28_devtype_data = {
> >
> > static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
> > .quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> > FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> > - FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> > FLEXCAN_QUIRK_BROKEN_PERR_STATE,
> > + FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> > FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> > + FLEXCAN_QUIRK_SETUP_STOP_MODE,
> > };
> >
> > static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
> > @@
> > -353,6 +368,35 @@ static inline void flexcan_write_le(u32 val, void
> > __iomem
> > *addr)
> > iowrite32(val, addr);
> > }
> >
> > +static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool
> > +enable) {
> > + struct flexcan_regs __iomem *regs = priv->regs;
> > + u32 reg_mcr;
> > +
> > + reg_mcr = priv->read(®s->mc
RE: [PATCH V4 1/1] can: flexcan: add self wakeup support
[...]
> +
> +static int __maybe_unused flexcan_noirq_suspend(struct device *device)
> +{
> + struct net_device *dev = dev_get_drvdata(device);
> + struct flexcan_priv *priv = netdev_priv(dev);
> +
> + if (netif_running(dev) && device_may_wakeup(device))
> + flexcan_enable_wakeup_irq(priv, true);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused flexcan_noirq_resume(struct device *device) {
> + struct net_device *dev = dev_get_drvdata(device);
> + struct flexcan_priv *priv = netdev_priv(dev);
> +
> + if (netif_running(dev) && device_may_wakeup(device)) {
> + disable_irq_wake(dev->irq);
A bit more thinking:
Can we put flexcan_enable_wakeup_irq(priv, false) here and move disable_irq_wake
to resume function?
Then it looks better on pairs for those functions.
I'm not sure if irq will be lost or we may even not need wakeup irq.
Please help check it.
Regards
Dong Aisheng
> + flexcan_exit_stop_mode(priv);
> }
> +
> return 0;
> }
>
> -static SIMPLE_DEV_PM_OPS(flexcan_pm_ops, flexcan_suspend,
> flexcan_resume);
> +static const struct dev_pm_ops flexcan_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(flexcan_suspend, flexcan_resume)
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(flexcan_noirq_suspend,
> +flexcan_noirq_resume) };
>
> static struct platform_driver flexcan_driver = {
> .driver = {
> --
> 2.17.1
RE: [PATCH V3 1/1] can: flexcan: Add self wakeup support
[...]
> >> @@
> >> -1437,10 +1552,18 @@ static int __maybe_unused flexcan_suspend(struct
> >> device *device)
> >>int err = 0;
> >>
> >>if (netif_running(dev)) {
> >> - err = flexcan_chip_disable(priv);
> >> - if (err)
> >> - return err;
> >> - err = pm_runtime_force_suspend(device);
> >> + /* if wakeup is enabled, enter stop mode
> >> + * else enter disabled mode.
> >> + */
> >> + if (device_may_wakeup(device)) {
> >> + enable_irq_wake(dev->irq);
> >> + flexcan_enter_stop_mode(priv);
> >> + } else {
> >> + err = flexcan_chip_disable(priv);
> >> + if (err)
> >> + return err;
> >> + err = pm_runtime_force_suspend(device);
> >
> > This is mixed with runtime pm.
> > I would suggestion you first upstream wakeup support.
> > Then runtime pm later separately.
>
> In fact this patch is based on the patch "can: flexcan: Implement CAN Runtime
> PM", which is still under review, too.
Yes, please ignore rpm patch first.
This patch will be re-made and not depends on rpm for easy review.
After wakeup is ready, rpm patch will rebase on it.
Regards
Dong Aisheng
>
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions| Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917- |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
RE: [PATCH V3 1/4] dt-bindings: rtc: add binding doc for i.MX system controller RTC driver
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-imx-sc.txt
> b/Documentation/devicetree/bindings/rtc/rtc-imx-sc.txt
> new file mode 100644
> index 000..d6e2353
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-imx-sc.txt
This should belong to scu binding doc.
Regards
Dong Aisheng
> @@ -0,0 +1,10 @@
> +* NXP i.MX System Controller Real Time Clock
> +
> +Required properties:
> +- compatible: should be "nxp,imx8qxp-sc-rtc";
> +
> +Example:
> +
> +rtc: rtc {
> + compatible = "nxp,imx8qxp-sc-rtc";
> +};
> --
> 2.7.4
RE: [PATCH V3 4/4] ARM64: dts: imx: add i.MX8QXP system controller RTC support
[...]
> Subject: [PATCH V3 4/4] ARM64: dts: imx: add i.MX8QXP system controller RTC
> support
>
> Add i.MX8QXP system controller RTC support.
>
> Signed-off-by: Anson Huang
> ---
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index da99b6f..5e8b554 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -406,4 +406,8 @@
> #size-cells = <1>;
> ranges = <0x5f00 0x0 0x5f00 0x100>;
> };
> +
> + rtc: rtc {
> + compatible = "nxp,imx8qxp-sc-rtc";
As I replied in patch 1, this belongs to scu node as well.
Regards
Dong Aisheng
> + };
> };
> --
> 2.7.4
RE: [PATCH V3 2/4] rtc: add i.MX system controller RTC support
> -Original Message-
> From: Anson Huang
> Sent: Wednesday, November 28, 2018 5:57 PM
[...]
> Subject: [PATCH V3 2/4] rtc: add i.MX system controller RTC support
>
> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller inside,
> the system controller is in charge of controlling power, clock and secure rtc
> etc..
>
> This patch adds i.MX system controller RTC driver support, Linux kernel has to
> communicate with system controller via MU (message unit) IPC to set/get RTC
> time and other alarm functions, since the RTC set time needs to be done in
> secure EL3 mode (required by system controller firmware) and alarm functions
> needs to be done with general MU IRQ handle, these depend on other
> components which are NOT ready, so this patch ONLY enables the RTC time
> read.
>
> Signed-off-by: Anson Huang
> ---
> changes since V2:
> - make rtc_ipc_handle/imx_sc_rtc static;
> - remove comma in last entry of imx_sc_dt_ids;
> - rename rtc_device.
> drivers/rtc/Kconfig | 6 +++
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-imx-sc.c | 107
This change log format seems strange.
> +++
> 3 files changed, 114 insertions(+)
> create mode 100644 drivers/rtc/rtc-imx-sc.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index a819ef0..3b9642e
> 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1677,6 +1677,12 @@ config RTC_DRV_SNVS
> This driver can also be built as a module, if so, the module
> will be called "rtc-snvs".
>
> +config RTC_DRV_IMX_SC
> + tristate "NXP i.MX System Controller RTC support"
> + help
> +If you say yes here you get support for the NXP i.MX System
> +Controller RTC module.
> +
> config RTC_DRV_SIRFSOC
> tristate "SiRFSOC RTC"
> depends on ARCH_SIRF
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index
> 290c173..f97c05e
> 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -75,6 +75,7 @@ obj-$(CONFIG_RTC_DRV_GOLDFISH) += rtc-goldfish.o
> obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
> obj-$(CONFIG_RTC_DRV_HYM8563)+= rtc-hym8563.o
> obj-$(CONFIG_RTC_DRV_IMXDI) += rtc-imxdi.o
> +obj-$(CONFIG_RTC_DRV_IMX_SC) += rtc-imx-sc.o
> obj-$(CONFIG_RTC_DRV_ISL12022) += rtc-isl12022.o
> obj-$(CONFIG_RTC_DRV_ISL12026) += rtc-isl12026.o
> obj-$(CONFIG_RTC_DRV_ISL1208)+= rtc-isl1208.o
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c new file mode
> 100644 index 000..b8e331e
> --- /dev/null
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018 NXP.
> + */
> +
> +#include
> +#include
> +#include
Do we need them all?
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define IMX_SC_TIMER_FUNC_GET_RTC_SEC19709
> +#define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
> +
> +static struct imx_sc_ipc *rtc_ipc_handle; static struct rtc_device
> +*imx_sc_rtc;
> +
> +struct imx_sc_msg_req_timer_get_rtc_time {
> + struct imx_sc_rpc_msg hdr;
> +} __packed;
> +
> +struct imx_sc_msg_resp_timer_get_rtc_time {
> + struct imx_sc_rpc_msg hdr;
> + u32 time;
> +} __packed;
> +
> +static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time
> +*tm) {
> + struct imx_sc_msg_resp_timer_get_rtc_time *resp;
> + struct imx_sc_msg_req_timer_get_rtc_time msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> + hdr->func = IMX_SC_TIMER_FUNC_GET_RTC_SEC1970;
> + hdr->size = 1;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + pr_err("read rtc time failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + resp = (struct imx_sc_msg_resp_timer_get_rtc_time *)&msg;
This is wrong and may result in memory corruption.
Please refer to how the scu-clk driver does.
Regards
Dong Aisheng
> + rtc_time_to_tm(resp->time, tm);
> +
> + return 0;
> +}
> +
> +static const struct rtc_class_ops imx_sc_rtc_ops = {
> + .read_time = imx_sc_rtc_read_time,
> +};
> +
> +static int imx_sc_rtc_probe(struct platform_device *pdev) {
> + int ret;
> +
> + ret = imx_scu_get_handle(&rtc_ipc_handle);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + dev_err(&pdev->dev, "failed to get ipc handle: %d!\n", ret);
> + return ret;
> + }
> +
> + imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> + if (IS_ERR(imx_sc_rtc)) {
> + ret = PTR_ERR(imx_sc_rtc);
> + return ret;
> + }
> +
> + imx_sc_rtc->ops = &imx_sc_rtc_ops;
> + imx_sc_rtc->range_min = 0;
> + imx_sc_rtc->range_max = U32_MAX;
> +
> + ret = rtc_register_device(imx_sc_rtc);
> + if (r
RE: [PATCH v13 0/5] Add i.MX8MQ clock driver
> -Original Message-
> From: Stephen Boyd [mailto:sb...@kernel.org]
[...]
> >
> > Changes since v12:
> > * replaced the division in clk_pll_recalc_rate in clk-frac
> >with do_div as suggested by Stephen
> >
> > Abel Vesa (2):
> > clk: imx: Add imx composite clock
> > clk: imx: Add clock driver for i.MX8MQ CCM
> >
> > Lucas Stach (3):
> > dt-bindings: add binding for i.MX8MQ CCM
> > clk: imx: add fractional PLL output clock
> > clk: imx: Add SCCG PLL type
> >
>
> I had to apply this set of fixes to silence sparse and smatch warnings about
> things that are not right. Please take a look over things and see if it's
> sane.
>
The change looks good to me and tested ok.
I did not see this patch series in your tree.
Do you want us to apply your changes and re-send for easy pick up?
Regards
Dong Aisheng
> diff --git a/drivers/clk/imx/clk-composite-8m.c
> b/drivers/clk/imx/clk-composite-8m.c
> index bcd31d889584..6d9d3714b4df 100644
> --- a/drivers/clk/imx/clk-composite-8m.c
> +++ b/drivers/clk/imx/clk-composite-8m.c
> @@ -127,8 +127,8 @@ struct clk *imx8m_clk_composite_flags(const char
> *name,
> int num_parents, void __iomem *reg,
> unsigned long flags)
> {
> - struct clk_hw *hw = NULL, *mux_hw = NULL;
> - struct clk_hw *div_hw = NULL, *gate_hw = NULL;
> + struct clk_hw *hw = ERR_PTR(-ENOMEM), *mux_hw;
> + struct clk_hw *div_hw, *gate_hw;
> struct clk_divider *div = NULL;
> struct clk_gate *gate = NULL;
> struct clk_mux *mux = NULL;
> diff --git a/drivers/clk/imx/clk-frac-pll.c b/drivers/clk/imx/clk-frac-pll.c
> index
> a3732be5ad7f..98726206f3c4 100644
> --- a/drivers/clk/imx/clk-frac-pll.c
> +++ b/drivers/clk/imx/clk-frac-pll.c
> @@ -14,6 +14,8 @@
> #include
> #include
>
> +#include "clk.h"
> +
> #define PLL_CFG0 0x0
> #define PLL_CFG1 0x4
>
> @@ -214,7 +216,7 @@ struct clk *imx_clk_frac_pll(const char *name, const
> char *parent_name,
> ret = clk_hw_register(NULL, hw);
> if (ret) {
> kfree(pll);
> - return ERR_CAST(hw);
> + return ERR_PTR(ret);
> }
>
> return hw->clk;
> diff --git a/drivers/clk/imx/clk-sccg-pll.c b/drivers/clk/imx/clk-sccg-pll.c
> index
> 4666b96bdb3f..ee7752bace89 100644
> --- a/drivers/clk/imx/clk-sccg-pll.c
> +++ b/drivers/clk/imx/clk-sccg-pll.c
> @@ -249,7 +249,7 @@ struct clk *imx_clk_sccg_pll(const char *name,
> ret = clk_hw_register(NULL, hw);
> if (ret) {
> kfree(pll);
> - return ERR_CAST(hw);
> + return ERR_PTR(ret);
> }
>
> return hw->clk;
RE: [PATCH V6 0/9] clk: add imx7ulp clk support
> -Original Message- > From: Stephen Boyd [mailto:sb...@kernel.org] > Sent: Tuesday, December 4, 2018 3:32 AM > To: Aisheng DONG ; linux-...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > mturque...@baylibre.com; shawn...@kernel.org; Anson Huang > ; Jacky Bai ; dl-linux-imx > ; Aisheng DONG > Subject: Re: [PATCH V6 0/9] clk: add imx7ulp clk support > > Quoting A.s. Dong (2018-11-14 05:01:31) > > This patch series intends to add imx7ulp clk support. > > > > i.MX7ULP Clock functions are under joint control of the System Clock > > Generation (SCG) modules, Peripheral Clock Control (PCC) modules, and > > Core Mode Controller (CMC)1 blocks > > > > The clocking scheme provides clear separation between M4 domain and A7 > > domain. Except for a few clock sources shared between two domains, > > such as the System Oscillator clock, the Slow IRC (SIRC), and and the > > Fast IRC clock (FIRCLK), clock sources and clock management are > > separated and contained within each domain. > > > > M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules. > > A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules. > > > > Note: this series only adds A7 clock domain support as M4 clock domain > > will be handled by M4 seperately. > > > > I got: > > drivers/clk/imx/clk-pllv4.c:152:15: warning: symbol 'imx_clk_pllv4' was not > declared. Should it be static? > drivers/clk/imx/clk-pfdv2.c:166:15: warning: symbol 'imx_clk_pfdv2' was not > declared. Should it be static? > drivers/clk/imx/clk-divider-gate.c:174:15: warning: symbol > 'imx_clk_divider_gate' was not declared. Should it be static? > drivers/clk/imx/clk-composite-7ulp.c:22:15: warning: symbol > 'imx7ulp_clk_composite' was not declared. Should it be static? > > which I can fix easily by throwing in clk.h into each file. Thanks, I will double check it when I back to office. Regards Dong Aisheng
RE: [PATCH 2/3] clk: imx: Build imx8mq clocks on arm64 only
> -Original Message- > From: Abel Vesa > Sent: Thursday, December 13, 2018 9:18 PM > Subject: [PATCH 2/3] clk: imx: Build imx8mq clocks on arm64 only > > The composite-8m, frac and sccg clocks are not used by any arm32 IMX SoC, > therefore build them only on arm64. CONFIG_ARCH_MXC is arm64 only, use > that. CONFIG_ARCH_MXC is not ARM64 only. > > Signed-off-by: Abel Vesa > --- > drivers/clk/imx/Makefile | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index > 62a1882..cc95744 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -3,13 +3,11 @@ > obj-y += \ > clk.o \ > clk-busy.o \ > - clk-composite-8m.o \ > clk-cpu.o \ > clk-composite-7ulp.o \ > clk-divider-gate.o \ > clk-fixup-div.o \ > clk-fixup-mux.o \ > - clk-frac-pll.o \ > clk-gate-exclusive.o \ > clk-gate2.o \ > clk-pfd.o \ > @@ -17,8 +15,7 @@ obj-y += \ > clk-pllv1.o \ > clk-pllv2.o \ > clk-pllv3.o \ > - clk-pllv4.o \ > - clk-sccg-pll.o > + clk-pllv4.o > > obj-$(CONFIG_SOC_IMX1) += clk-imx1.o > obj-$(CONFIG_SOC_IMX21) += clk-imx21.o @@ -36,4 +33,7 @@ > obj-$(CONFIG_SOC_IMX7D) += clk-imx7d.o > obj-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o > obj-$(CONFIG_SOC_VF610) += clk-vf610.o > > -obj-$(CONFIG_ARCH_MXC) += clk-imx8mq.o > +obj-$(CONFIG_ARCH_MXC) += clk-imx8mq.o \ > + clk-composite-8m.o \ > + clk-frac-pll.o \ > + clk-sccg-pll.o I think you may create a CLK_IMX8MQ as I said in other mail for IMX8QXP. Regards Dong Aisheng > -- > 2.7.4
RE: [PATCH] clk: imx: Make the i.MX8MQ CCM clock driver CLK_IMX8MQ dependant
[...] > > --- a/drivers/clk/imx/Makefile > > +++ b/drivers/clk/imx/Makefile > > @@ -34,5 +34,6 @@ obj-$(CONFIG_SOC_IMX6SX) += clk-imx6sx.o > > obj-$(CONFIG_SOC_IMX6UL) += clk-imx6ul.o > > obj-$(CONFIG_SOC_IMX7D) += clk-imx7d.o > > obj-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o > > -obj-$(CONFIG_SOC_IMX8MQ) += clk-imx8mq.o > > obj-$(CONFIG_SOC_VF610) += clk-vf610.o > > + > > +obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o > > Nit: Do we want to keep CONFIG_ sorted? IMHO It might be okay to make MX8 (ARM64) a new group to start To get a clear separation. (Slightly not sorted due to VF610) Anyway, it leaves to Stephen to make the judge. Regards Dong Aisheng
RE: [PATCH V4 2/4] rtc: add i.MX system controller RTC support
[...] > > > > I applied it on my test branch and this makes sparc64-allyesconfig > > fail > > with: > > > >drivers/rtc/rtc-imx-sc.o: In function `imx_sc_rtc_read_time': > >rtc-imx-sc.c:(.text+0x34): undefined reference to `imx_scu_call_rpc' > >drivers/rtc/rtc-imx-sc.o: In function `imx_sc_rtc_probe': > >rtc-imx-sc.c:(.text+0x90): undefined reference to `imx_scu_get_handle' > > Maybe there are some changes on basic support of imx scu driver, I will resend > a patch set later, thanks. > This driver should depend on IMX_SCU which seems not got compiled. Regards Dong Aisheng > Anson. > > > > > > > > > -- > > Alexandre Belloni, Bootlin > > Embedded Linux and Kernel engineering > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fboo > > tl > in.com&data=02%7C01%7Canson.huang%40nxp.com%7C72b147643a3f > > > 44c258c208d6658f9cce%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > > %7C636808064818797711&sdata=%2Bl5jGXX6wBYbOg5Zalg4CyxyZKj > Ue > > uzRSQEKO9TJnRI%3D&reserved=0
RE: [PATCH V4 2/4] rtc: add i.MX system controller RTC support
> -Original Message-
> From: Anson Huang
> Sent: Thursday, November 29, 2018 9:50 AM
[...]
> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller inside,
> the system controller is in charge of controlling power, clock and secure rtc
> etc..
>
> This patch adds i.MX system controller RTC driver support, Linux kernel has to
> communicate with system controller via MU (message unit) IPC to set/get RTC
> time and other alarm functions, since the RTC set time needs to be done in
> secure EL3 mode (required by system controller firmware) and alarm functions
> needs to be done with general MU IRQ handle, these depend on other
> components which are NOT ready, so this patch ONLY enables the RTC time
> read.
>
> Signed-off-by: Anson Huang
> ---
> ChangeLog:
> V3->V4:
> *remove unnecessary headfile included;
> *fix potential memory currpution for the message responce;
> *update the compatible string name to start with "fsl" instead of "nxp".
> drivers/rtc/Kconfig | 6 +++
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-imx-sc.c | 98
>
> 3 files changed, 105 insertions(+)
> create mode 100644 drivers/rtc/rtc-imx-sc.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index a819ef0..3b9642e
> 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1677,6 +1677,12 @@ config RTC_DRV_SNVS
> This driver can also be built as a module, if so, the module
> will be called "rtc-snvs".
>
> +config RTC_DRV_IMX_SC
> + tristate "NXP i.MX System Controller RTC support"
This should depend on IMX_SCU
> + help
> +If you say yes here you get support for the NXP i.MX System
> +Controller RTC module.
> +
> config RTC_DRV_SIRFSOC
> tristate "SiRFSOC RTC"
> depends on ARCH_SIRF
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index
> 290c173..f97c05e
> 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -75,6 +75,7 @@ obj-$(CONFIG_RTC_DRV_GOLDFISH) += rtc-goldfish.o
> obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
> obj-$(CONFIG_RTC_DRV_HYM8563)+= rtc-hym8563.o
> obj-$(CONFIG_RTC_DRV_IMXDI) += rtc-imxdi.o
> +obj-$(CONFIG_RTC_DRV_IMX_SC) += rtc-imx-sc.o
> obj-$(CONFIG_RTC_DRV_ISL12022) += rtc-isl12022.o
> obj-$(CONFIG_RTC_DRV_ISL12026) += rtc-isl12026.o
> obj-$(CONFIG_RTC_DRV_ISL1208)+= rtc-isl1208.o
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c new file mode
> 100644 index 000..5976a2e
> --- /dev/null
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018 NXP.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define IMX_SC_TIMER_FUNC_GET_RTC_SEC19709
> +#define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
> +
> +static struct imx_sc_ipc *rtc_ipc_handle; static struct rtc_device
> +*imx_sc_rtc;
> +
> +struct imx_sc_msg_timer_get_rtc_time {
> + struct imx_sc_rpc_msg hdr;
> + u32 time;
> +} __packed;
> +
> +static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time
> +*tm) {
> + struct imx_sc_msg_timer_get_rtc_time msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> + hdr->func = IMX_SC_TIMER_FUNC_GET_RTC_SEC1970;
> + hdr->size = 1;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + pr_err("read rtc time failed, ret %d\n", ret);
Nitpick:
better to use dev_err?
> + return ret;
> + }
> +
> + rtc_time_to_tm(msg.time, tm);
> +
> + return 0;
> +}
> +
> +static const struct rtc_class_ops imx_sc_rtc_ops = {
> + .read_time = imx_sc_rtc_read_time,
> +};
> +
> +static int imx_sc_rtc_probe(struct platform_device *pdev) {
> + int ret;
> +
> + ret = imx_scu_get_handle(&rtc_ipc_handle);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + dev_err(&pdev->dev, "failed to get ipc handle: %d!\n", ret);
> + return ret;
> + }
You don't have to handle different error cases as API has already enclosed it.
> +
> + imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> + if (IS_ERR(imx_sc_rtc)) {
> + ret = PTR_ERR(imx_sc_rtc);
> + return ret;
Return PTR_ERR(imx_sc_rtc)
Regards
Dong Aisheng
> + }
> +
> + imx_sc_rtc->ops = &imx_sc_rtc_ops;
> + imx_sc_rtc->range_min = 0;
> + imx_sc_rtc->range_max = U32_MAX;
> +
> + ret = rtc_register_device(imx_sc_rtc);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register rtc: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id imx_sc_dt_ids[] = {
> + { .compatible = "fsl,imx8qxp-sc-rtc", },
> + {}
> +};
> +MODULE_
RE: [PATCH v3 3/3] reset: imx7: Add support for i.MX8MQ IP block variant
> -Original Message-
> From: Andrey Smirnov [mailto:andrew.smir...@gmail.com]
> Sent: Monday, December 17, 2018 10:38 AM
> To: linux-kernel@vger.kernel.org
> Cc: Andrey Smirnov ; p.za...@pengutronix.de;
> Fabio Estevam ; cphe...@gmail.com;
> l.st...@pengutronix.de; Leonard Crestez ; Aisheng
> Dong ; Richard Zhu ; Rob
> Herring ; devicet...@vger.kernel.org; dl-linux-imx
> ; linux-arm-ker...@lists.infradead.org
> Subject: [PATCH v3 3/3] reset: imx7: Add support for i.MX8MQ IP block variant
>
> Add bits and pieces needed to support IP block variant found on i.MX8MQ
> SoCs.
>
> Cc: p.za...@pengutronix.de
> Cc: Fabio Estevam
> Cc: cphe...@gmail.com
> Cc: l.st...@pengutronix.de
> Cc: Leonard Crestez
> Cc: "A.s. Dong"
> Cc: Richard Zhu
> Cc: Rob Herring
> Cc: devicet...@vger.kernel.org
> Cc: linux-...@nxp.com
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov
> ---
> drivers/reset/Kconfig | 2 +-
> drivers/reset/reset-imx7.c | 106
> +
> 2 files changed, 107 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index
> c21da9fe51ec..4909aab7401b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -50,7 +50,7 @@ config RESET_HSDK
> config RESET_IMX7
> bool "i.MX7 Reset Driver" if COMPILE_TEST
> depends on HAS_IOMEM
> - default SOC_IMX7D
> + default SOC_IMX7D || SOC_IMX8MQ
SOC_IMX8MQ has been removed in Shawn's tree.
I'd suggest simply using ARCH_MXC.
Regards
Dong Aisheng
> select MFD_SYSCON
> help
> This enables the reset controller driver for i.MX7 SoCs.
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c index
> 3a36d5863891..bb826935db6d 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
>
> struct imx7_src_signal {
> unsigned int offset, bit;
> @@ -113,6 +114,110 @@ static const struct imx7_src_variant variant_imx7 =
> {
> .prepare = imx7_src_prepare,
> };
>
> +enum imx8mq_src_registers {
> + SRC_A53RCR0 = 0x0004,
> + SRC_HDMI_RCR= 0x0030,
> + SRC_DISP_RCR= 0x0034,
> + SRC_GPU_RCR = 0x0040,
> + SRC_VPU_RCR = 0x0044,
> + SRC_PCIE2_RCR = 0x0048,
> + SRC_MIPIPHY1_RCR= 0x004c,
> + SRC_MIPIPHY2_RCR= 0x0050,
> + SRC_DDRC2_RCR = 0x1004,
> +};
> +
> +static const struct imx7_src_signal
> imx8mq_src_signals[IMX8MQ_RESET_NUM] = {
> + [IMX8MQ_RESET_A53_CORE_POR_RESET0] = { SRC_A53RCR0, BIT(0) },
> + [IMX8MQ_RESET_A53_CORE_POR_RESET1] = { SRC_A53RCR0, BIT(1) },
> + [IMX8MQ_RESET_A53_CORE_POR_RESET2] = { SRC_A53RCR0, BIT(2) },
> + [IMX8MQ_RESET_A53_CORE_POR_RESET3] = { SRC_A53RCR0, BIT(3) },
> + [IMX8MQ_RESET_A53_CORE_RESET0] = { SRC_A53RCR0, BIT(4) },
> + [IMX8MQ_RESET_A53_CORE_RESET1] = { SRC_A53RCR0, BIT(5) },
> + [IMX8MQ_RESET_A53_CORE_RESET2] = { SRC_A53RCR0, BIT(6) },
> + [IMX8MQ_RESET_A53_CORE_RESET3] = { SRC_A53RCR0, BIT(7) },
> + [IMX8MQ_RESET_A53_DBG_RESET0] = { SRC_A53RCR0, BIT(8) },
> + [IMX8MQ_RESET_A53_DBG_RESET1] = { SRC_A53RCR0, BIT(9) },
> + [IMX8MQ_RESET_A53_DBG_RESET2] = { SRC_A53RCR0, BIT(10) },
> + [IMX8MQ_RESET_A53_DBG_RESET3] = { SRC_A53RCR0, BIT(11) },
> + [IMX8MQ_RESET_A53_ETM_RESET0] = { SRC_A53RCR0, BIT(12) },
> + [IMX8MQ_RESET_A53_ETM_RESET1] = { SRC_A53RCR0, BIT(13) },
> + [IMX8MQ_RESET_A53_ETM_RESET2] = { SRC_A53RCR0, BIT(14) },
> + [IMX8MQ_RESET_A53_ETM_RESET3] = { SRC_A53RCR0, BIT(15) },
> + [IMX8MQ_RESET_A53_SOC_DBG_RESET]= { SRC_A53RCR0, BIT(20) },
> + [IMX8MQ_RESET_A53_L2RESET] = { SRC_A53RCR0, BIT(21) },
> + [IMX8MQ_RESET_SW_NON_SCLR_M4C_RST] = { SRC_M4RCR, BIT(0) },
> + [IMX8MQ_RESET_OTG1_PHY_RESET] = { SRC_USBOPHY1_RCR,
> BIT(0) },
> + [IMX8MQ_RESET_OTG2_PHY_RESET] = { SRC_USBOPHY2_RCR,
> BIT(0) },
> + [IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N]= { SRC_MIPIPHY_RCR,
> BIT(1) },
> + [IMX8MQ_RESET_MIPI_DSI_RESET_N] = { SRC_MIPIPHY_RCR,
> BIT(2) },
> + [IMX8MQ_RESET_MIPI_DIS_DPI_RESET_N] = { SRC_MIPIPHY_RCR,
> BIT(3) },
> + [IMX8MQ_RESET_MIPI_DIS_ESC_RESET_N] = { SRC_MIPIPHY_RCR,
> BIT(4) },
> + [IMX8MQ_RESET_MIPI_DIS_PCLK_RESET_N]= { SRC_MIPIPHY_RCR
RE: [PATCH 1/2] Documentation: can: flexcan: add PE clock source property to device tree
> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Friday, December 21, 2018 4:23 AM
>
> On Thu, Dec 13, 2018 at 07:07:57AM +, Joakim Zhang wrote:
> > From: Dong Aisheng
> >
> > The FlexCAN controller can parse clock source property from DTS file
> > to select PE clock source.
> >
> > Signed-off-by: Dong Aisheng
> > Signed-off-by: Joakim Zhang
> > ---
> > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8
> >
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > index bc77477c6878..a04168605998 100644
> > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > @@ -32,6 +32,13 @@ Optional properties:
> > ack_gpr is the gpr register offset of CAN stop acknowledge.
> > ack_bit is the bit offset of CAN stop acknowledge.
> >
> > +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE).
> > + It's SoC Implementation dependent. Refer to RM for detailed
>
> If SoC dependent, then it should be implied by the SoC specific compatible.
> Also, seems like you should add clock binding support here if you need more
> clock control.
The clock source selection is done by a register bit inside the IP block:
BIT13 CLKSRC CAN Engine Clock Source
0b - The CAN engine clock source is the oscillator clock.
1b - The CAN engine clock source is the peripheral clock.
Currently it's written 1 by default during driver initialization.
drivers/net/can/flexcan.c
/* select "bus clock", chip must be disabled */
reg = priv->read(®s->ctrl);
reg |= FLEXCAN_CTRL_CLK_SRC;
priv->write(reg, ®s->ctrl);
I'm not sure if it's a typical case to abstract CLKSRC bit into a common clock
mux.
(Is there any similar case in kernel?)
But I think we can also use SoC specific compatible to write 0 for those special
Ones (currently only imx8qxp). Then this patch may not be needed.
Marc,
Please let us know if you have a different idea.
Regards
Dong Aisheng
>
> > + definition. If this property is not set in device tree node
> > + then driver selects clock source 1 by default.
> > + 0: clock source 0 (oscillator clock)
> > + 1: clock source 1 (peripheral clock)
> > +
> > Example:
> >
> > can@1c000 {
> > @@ -40,4 +47,5 @@ Example:
> > interrupts = <48 0x2>;
> > interrupt-parent = <&mpic>;
> > clock-frequency = <2>; // filled in by bootloader
> > + fsl,clk-source = <0>; // select clock source 0 for PE
> > };
> > --
> > 2.17.1
> >
RE: [PATCH] pinctrl: freescale: Break dependency on SOC_IMX8MQ for i.MX8MQ
> -Original Message- > From: Abel Vesa > Sent: Sunday, December 23, 2018 3:09 PM > > The CONFIG_SOC_IMX8MQ will go away, so the dependency can be based on > ARCH_MXC && ARM64. > > Signed-off-by: Abel Vesa Acked-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH] phy: freescale: Break dependency on SOC_IMX8MQ for USB PHY
> -Original Message- > From: Abel Vesa > Sent: Sunday, December 23, 2018 3:04 PM > To: Kishon Vijay Abraham I ; Jun Li ; Lucas > Stach > Cc: dl-linux-imx ; Linux Kernel Mailing List > ; Abel Vesa > Subject: [PATCH] phy: freescale: Break dependency on SOC_IMX8MQ for USB > PHY > > Since this is going to be used on more SoCs than just i.MX8MQ, make the > dependency here more generic. > > Signed-off-by: Abel Vesa Reviewed-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH] soc: imx: Break dependency on SOC_IMX8MQ for GPCv2
> -Original Message- > From: Abel Vesa > Sent: Sunday, December 23, 2018 3:00 PM > To: Shawn Guo ; Sascha Hauer > ; Fabio Estevam > Cc: dl-linux-imx ; linux-arm-ker...@lists.infradead.org; > Linux Kernel Mailing List ; Abel Vesa > > Subject: [PATCH] soc: imx: Break dependency on SOC_IMX8MQ for GPCv2 > > Since this is going to be used on more SoCs than just i.MX8MQ, make the > dependency here more generic. > > Signed-off-by: Abel Vesa > --- > drivers/soc/imx/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index > 2112d18..7ffbb6b 100644 > --- a/drivers/soc/imx/Kconfig > +++ b/drivers/soc/imx/Kconfig > @@ -2,7 +2,7 @@ menu "i.MX SoC drivers" > > config IMX_GPCV2_PM_DOMAINS > bool "i.MX GPCv2 PM domains" > - depends on SOC_IMX7D || SOC_IMX8MQ || (COMPILE_TEST && OF) > + depends on SOC_IMX7D || ARCH_MXC || (COMPILE_TEST && OF) Nitpick: ARCH_MXC already contains SOC_IMX7D, so simply depends on ARCH_MXC || (COMPILE_TEST && OF) should be enough. Otherwise: Reviewed-by: Dong Aisheng Regards Dong Aisheng > depends on PM > select PM_GENERIC_DOMAINS > default y if SOC_IMX7D > -- > 2.7.4
RE: [RFC PATCH 1/1] PM / Domains: Add multi PM domains support for attach_dev
> -Original Message-
> From: Ulf Hansson [mailto:ulf.hans...@linaro.org]
> Sent: Wednesday, January 2, 2019 6:49 PM
>
> On Sat, 29 Dec 2018 at 07:43, Aisheng Dong wrote:
> >
> > > From: Ulf Hansson [mailto:ulf.hans...@linaro.org]
> > > Sent: Friday, December 28, 2018 11:37 PM
> > >
> > > On Thu, 27 Dec 2018 at 18:14, Aisheng Dong
> > > wrote:
> > > >
> > > > Currently attach_dev() in power domain infrastructure still does
> > > > not support multi domains case as the struct device *dev passed
> > > > down from
> > > > genpd_dev_pm_attach_by_id() is a virtual PD device, it does not
> > > > help for parsing the real device information from device tree, e.g.
> > > > Device/Power IDs, Clocks and it's unware of which real power
> > > > domain the device should attach.
> > >
> > > Thanks for working on this!
> > >
> > > I would appreciate if the changelog could clarify the problem a bit.
> > > Perhaps something along the lines of the below.
> > >
> >
> > Sounds good to me.
> > I will add them in commit message.
> > Thanks for the suggestion.
> >
> > > "A genpd provider's ->attach_dev() callback may be invoked with a so
> > > called virtual device, which is created by genpd, at the point when
> > > a device is being attached to one of its corresponding multiple PM
> domains.
> > >
> > > In these cases, the genpd provider fails to look up any resource, by
> > > a
> > > clk_get() for example, for the virtual device in question. This is
> > > because, the virtual device that was created by genpd, does not have
> > > the virt_dev->of_node assigned."
> > >
> > > >
> > > > Extend the framework a bit to store the multi PM domains
> > > > information in per-device struct generic_pm_domain_data, then
> > > > power domain driver could retrieve it for necessary operations during
> attach_dev().
> > > >
> > > > Two new APIs genpd_is_mpd_device() and dev_gpd_mpd_data() are also
> > > > introduced to ease the driver operation.
> > > >
> > > > Cc: "Rafael J. Wysocki"
> > > > Cc: Kevin Hilman
> > > > Cc: Ulf Hansson
> > > > Cc: Greg Kroah-Hartman
> > > > Signed-off-by: Dong Aisheng
> > > > ---
> > > > This patch is a follow-up work of the earlier discussion with Ulf
> > > > Hansson about the multi PM domains support for the attach_dev()
> > > > function
> > > [1].
> > > > After a bit more thinking, this is a less intrusive implementation
> > > > with the mininum impact on the exist function definitions and
> > > > calling
> > > follows.
> > > > One known little drawback is that we have to use the device driver
> > > > private data (device.drvdata) to pass down the multi domains
> > > > information in a earlier time. However, as multi PD devices are
> > > > created by domain framework, this seems to be safe to use it in
> > > > domain core code as device driver is not likely going to use it.
> > > > Anyway, if any better ideas, please let me know.
> > > >
> > > > With the two new APIs, the using can be simply as:
> > > > static int xxx_attach_dev(struct generic_pm_domain *domain,
> > > > struct device *dev) {
> > > > ...
> > > > if (genpd_is_mpd_device(dev)) {
> > > > mpd_data = dev_gpd_mpd_data(dev);
> > > > np = mpd_data->parent->of_node;
> > > > idx = mpd_data->index;
> > > > //dts parsing
> > > > ...
> > > > }
> > > > ...
> > >
> > > I think we can make this a lot less complicated. Just assign
> > > virt_dev->of_node = of_node_get(dev->of_node), somewhere in
> > > genpd_dev_pm_attach_by_id() and before calling
> __genpd_dev_pm_attach().
> > >
> > > Doing that, would mean the genpd provider's ->attach_dev() callback,
> > > don't have to distinguish between virtual and non-virtual devices.
> > > Instead they should be able to look up resources in the same way as
> > > they did before.
> > >
> >
> > Yes, that seems like a smart way.
> > But there's still a remain problem that how about th
RE: [PATCH] clk: imx: Remove Kconfig duplicate include
> From: Abel Vesa
> Sent: Friday, January 4, 2019 12:58 AM
> Subject: [PATCH] clk: imx: Remove Kconfig duplicate include
>
> Commit d360b130e210f2 ("clk: imx: Make the i.MX8MQ CCM clock driver
> CLK_IMX8MQ dependant") introduced this dupplicate (and wrongfuly
> ordered).
>
> Fixes: d360b130e210f2 ("clk: imx: Make the i.MX8MQ CCM clock driver
> CLK_IMX8MQ dependant")
> Signed-off-by: Abel Vesa
Reviewed-by: Dong Aisheng
Regards
Dong Aisheng
> ---
> drivers/clk/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index e5b2fe8..d2f0bb5
> 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -293,7 +293,6 @@ config COMMON_CLK_BD718XX source
> "drivers/clk/actions/Kconfig"
> source "drivers/clk/bcm/Kconfig"
> source "drivers/clk/hisilicon/Kconfig"
> -source "drivers/clk/imx/Kconfig"
> source "drivers/clk/imgtec/Kconfig"
> source "drivers/clk/imx/Kconfig"
> source "drivers/clk/ingenic/Kconfig"
> --
> 2.7.4
RE: [RFC PATCH 1/1] PM / Domains: Add multi PM domains support for attach_dev
> From: Ulf Hansson [mailto:ulf.hans...@linaro.org]
> Sent: Friday, January 4, 2019 5:11 AM
> On Wed, 2 Jan 2019 at 17:29, Aisheng Dong wrote:
> >
> > > -Original Message-
> > > From: Ulf Hansson [mailto:ulf.hans...@linaro.org]
> > > Sent: Wednesday, January 2, 2019 6:49 PM
> > >
> > > On Sat, 29 Dec 2018 at 07:43, Aisheng Dong
> wrote:
> > > >
> > > > > From: Ulf Hansson [mailto:ulf.hans...@linaro.org]
> > > > > Sent: Friday, December 28, 2018 11:37 PM
> > > > >
> > > > > On Thu, 27 Dec 2018 at 18:14, Aisheng Dong
> > > > >
> > > > > wrote:
> > > > > >
> > > > > > Currently attach_dev() in power domain infrastructure still
> > > > > > does not support multi domains case as the struct device *dev
> > > > > > passed down from
> > > > > > genpd_dev_pm_attach_by_id() is a virtual PD device, it does
> > > > > > not help for parsing the real device information from device tree,
> > > > > > e.g.
> > > > > > Device/Power IDs, Clocks and it's unware of which real power
> > > > > > domain the device should attach.
> > > > >
> > > > > Thanks for working on this!
> > > > >
> > > > > I would appreciate if the changelog could clarify the problem a bit.
> > > > > Perhaps something along the lines of the below.
> > > > >
> > > >
> > > > Sounds good to me.
> > > > I will add them in commit message.
> > > > Thanks for the suggestion.
> > > >
> > > > > "A genpd provider's ->attach_dev() callback may be invoked with
> > > > > a so called virtual device, which is created by genpd, at the
> > > > > point when a device is being attached to one of its
> > > > > corresponding multiple PM
> > > domains.
> > > > >
> > > > > In these cases, the genpd provider fails to look up any
> > > > > resource, by a
> > > > > clk_get() for example, for the virtual device in question. This
> > > > > is because, the virtual device that was created by genpd, does
> > > > > not have the virt_dev->of_node assigned."
> > > > >
> > > > > >
> > > > > > Extend the framework a bit to store the multi PM domains
> > > > > > information in per-device struct generic_pm_domain_data, then
> > > > > > power domain driver could retrieve it for necessary operations
> > > > > > during
> > > attach_dev().
> > > > > >
> > > > > > Two new APIs genpd_is_mpd_device() and dev_gpd_mpd_data() are
> > > > > > also introduced to ease the driver operation.
> > > > > >
> > > > > > Cc: "Rafael J. Wysocki"
> > > > > > Cc: Kevin Hilman
> > > > > > Cc: Ulf Hansson
> > > > > > Cc: Greg Kroah-Hartman
> > > > > > Signed-off-by: Dong Aisheng
> > > > > > ---
> > > > > > This patch is a follow-up work of the earlier discussion with
> > > > > > Ulf Hansson about the multi PM domains support for the
> > > > > > attach_dev() function
> > > > > [1].
> > > > > > After a bit more thinking, this is a less intrusive
> > > > > > implementation with the mininum impact on the exist function
> > > > > > definitions and calling
> > > > > follows.
> > > > > > One known little drawback is that we have to use the device
> > > > > > driver private data (device.drvdata) to pass down the multi
> > > > > > domains information in a earlier time. However, as multi PD
> > > > > > devices are created by domain framework, this seems to be safe
> > > > > > to use it in domain core code as device driver is not likely going
> > > > > > to use
> it.
> > > > > > Anyway, if any better ideas, please let me know.
> > > > > >
> > > > > > With the two new APIs, the using can be simply as:
> > > > > > static int xxx_attach_dev(struct generic_pm_domain *domain,
> > > > > > struct device *dev) {
> > > > > > ...
> > > > > > if (genpd_is_mpd_device
RE: [PATCH] clk: imx: imx7ulp: use struct_size() in kzalloc()
> -Original Message-
> From: Gustavo A. R. Silva [mailto:gust...@embeddedor.com]
> Sent: Monday, December 24, 2018 2:40 PM
>
> One of the more common cases of allocation size calculations is finding the
> size of a structure that has a zero-sized array at the end, along with memory
> for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> void *entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva
Reviewed-by: Dong Aisheng
Regards
Dong Aisheng
[RFC PATCH 1/1] PM / Domains: Add multi PM domains support for attach_dev
Currently attach_dev() in power domain infrastructure still does
not support multi domains case as the struct device *dev passed
down from genpd_dev_pm_attach_by_id() is a virtual PD device, it
does not help for parsing the real device information from device
tree, e.g. Device/Power IDs, Clocks and it's unware of which real
power domain the device should attach.
Extend the framework a bit to store the multi PM domains information
in per-device struct generic_pm_domain_data, then power domain driver
could retrieve it for necessary operations during attach_dev().
Two new APIs genpd_is_mpd_device() and dev_gpd_mpd_data() are also
introduced to ease the driver operation.
Cc: "Rafael J. Wysocki"
Cc: Kevin Hilman
Cc: Ulf Hansson
Cc: Greg Kroah-Hartman
Signed-off-by: Dong Aisheng
---
This patch is a follow-up work of the earlier discussion with Ulf Hansson
about the multi PM domains support for the attach_dev() function [1].
After a bit more thinking, this is a less intrusive implementation with
the mininum impact on the exist function definitions and calling follows.
One known little drawback is that we have to use the device driver private
data (device.drvdata) to pass down the multi domains information in a
earlier time. However, as multi PD devices are created by domain framework,
this seems to be safe to use it in domain core code as device driver
is not likely going to use it.
Anyway, if any better ideas, please let me know.
With the two new APIs, the using can be simply as:
static int xxx_attach_dev(struct generic_pm_domain *domain,
struct device *dev)
{
...
if (genpd_is_mpd_device(dev)) {
mpd_data = dev_gpd_mpd_data(dev);
np = mpd_data->parent->of_node;
idx = mpd_data->index;
//dts parsing
...
}
...
}
[1] https://patchwork.kernel.org/patch/10658669/
---
drivers/base/power/domain.c | 31 +++
include/linux/pm_domain.h | 23 +++
2 files changed, 54 insertions(+)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 7f38a92..1aa0918 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1343,6 +1343,9 @@ static struct generic_pm_domain_data
*genpd_alloc_dev_data(struct device *dev,
gpd_data->td.effective_constraint_ns =
PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier;
+ if (genpd_is_mpd_device(dev))
+ gpd_data->mpd_data = dev_get_drvdata(dev);
+
spin_lock_irq(&dev->power.lock);
if (dev->power.subsys_data->domain_data) {
@@ -2179,6 +2182,7 @@ EXPORT_SYMBOL_GPL(of_genpd_remove_last);
static void genpd_release_dev(struct device *dev)
{
+ kfree(dev->driver_data);
kfree(dev);
}
@@ -2320,6 +2324,20 @@ int genpd_dev_pm_attach(struct device *dev)
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
/**
+ * genpd_is_mpd_device - Check if a device is associated with multi PM domains
+ * @dev: Device to check.
+ */
+
+bool genpd_is_mpd_device(struct device *dev)
+{
+ if (!dev || (dev && !dev->bus))
+ return false;
+
+ return dev->bus == &genpd_bus_type;
+};
+EXPORT_SYMBOL_GPL(genpd_is_mpd_device);
+
+/**
* genpd_dev_pm_attach_by_id - Associate a device with one of its PM domains.
* @dev: The device used to lookup the PM domain.
* @index: The index of the PM domain.
@@ -2338,6 +2356,7 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
struct device *genpd_dev_pm_attach_by_id(struct device *dev,
unsigned int index)
{
+ struct pm_domain_mpd_data *mpd_data;
struct device *genpd_dev;
int num_domains;
int ret;
@@ -2366,6 +2385,18 @@ struct device *genpd_dev_pm_attach_by_id(struct device
*dev,
return ERR_PTR(ret);
}
+ /* Allocate multi power domains data */
+ mpd_data = kzalloc(sizeof(*mpd_data), GFP_KERNEL);
+ if (!mpd_data) {
+ device_unregister(genpd_dev);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ mpd_data->parent = dev;
+ mpd_data->index = index;
+
+ dev_set_drvdata(genpd_dev, mpd_data);
+
/* Try to attach the device to the PM domain at the specified index. */
ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index, false);
if (ret < 1) {
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 3b5d728..106d4e7 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -144,6 +144,11 @@ struct gpd_timing_data {
bool cached_suspend_ok;
};
+struct pm_domain_mpd_data {
+ struct device *parent;
+ unsigned int index;
+};
+
struct pm_domain_data {
struct list_head list_node;
struct device *dev;
@@ -151,6 +156,7 @@ struct pm_domain_data {
struct generic_pm_domain_data {
struct pm_do
RE: [RFC PATCH 1/1] PM / Domains: Add multi PM domains support for attach_dev
> From: Ulf Hansson [mailto:ulf.hans...@linaro.org]
> Sent: Friday, December 28, 2018 11:37 PM
>
> On Thu, 27 Dec 2018 at 18:14, Aisheng Dong
> wrote:
> >
> > Currently attach_dev() in power domain infrastructure still does not
> > support multi domains case as the struct device *dev passed down from
> > genpd_dev_pm_attach_by_id() is a virtual PD device, it does not help
> > for parsing the real device information from device tree, e.g.
> > Device/Power IDs, Clocks and it's unware of which real power domain
> > the device should attach.
>
> Thanks for working on this!
>
> I would appreciate if the changelog could clarify the problem a bit.
> Perhaps something along the lines of the below.
>
Sounds good to me.
I will add them in commit message.
Thanks for the suggestion.
> "A genpd provider's ->attach_dev() callback may be invoked with a so called
> virtual device, which is created by genpd, at the point when a device is being
> attached to one of its corresponding multiple PM domains.
>
> In these cases, the genpd provider fails to look up any resource, by a
> clk_get() for example, for the virtual device in question. This is because,
> the
> virtual device that was created by genpd, does not have the virt_dev->of_node
> assigned."
>
> >
> > Extend the framework a bit to store the multi PM domains information
> > in per-device struct generic_pm_domain_data, then power domain driver
> > could retrieve it for necessary operations during attach_dev().
> >
> > Two new APIs genpd_is_mpd_device() and dev_gpd_mpd_data() are also
> > introduced to ease the driver operation.
> >
> > Cc: "Rafael J. Wysocki"
> > Cc: Kevin Hilman
> > Cc: Ulf Hansson
> > Cc: Greg Kroah-Hartman
> > Signed-off-by: Dong Aisheng
> > ---
> > This patch is a follow-up work of the earlier discussion with Ulf
> > Hansson about the multi PM domains support for the attach_dev() function
> [1].
> > After a bit more thinking, this is a less intrusive implementation
> > with the mininum impact on the exist function definitions and calling
> follows.
> > One known little drawback is that we have to use the device driver
> > private data (device.drvdata) to pass down the multi domains
> > information in a earlier time. However, as multi PD devices are
> > created by domain framework, this seems to be safe to use it in domain
> > core code as device driver is not likely going to use it.
> > Anyway, if any better ideas, please let me know.
> >
> > With the two new APIs, the using can be simply as:
> > static int xxx_attach_dev(struct generic_pm_domain *domain,
> > struct device *dev) {
> > ...
> > if (genpd_is_mpd_device(dev)) {
> > mpd_data = dev_gpd_mpd_data(dev);
> > np = mpd_data->parent->of_node;
> > idx = mpd_data->index;
> > //dts parsing
> > ...
> > }
> > ...
>
> I think we can make this a lot less complicated. Just assign
> virt_dev->of_node =
> of_node_get(dev->of_node), somewhere in
> genpd_dev_pm_attach_by_id() and before calling __genpd_dev_pm_attach().
>
> Doing that, would mean the genpd provider's ->attach_dev() callback, don't
> have to distinguish between virtual and non-virtual devices.
> Instead they should be able to look up resources in the same way as they did
> before.
>
Yes, that seems like a smart way.
But there's still a remain problem that how about the domain index information
needed for attach_dev()?
Regards
Dong Aisheng
> Kind regards
> Uffe
>
> > }
> >
> > ---
> > drivers/base/power/domain.c | 31 +++
> > include/linux/pm_domain.h | 23 +++
> > 2 files changed, 54 insertions(+)
> >
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index 7f38a92..1aa0918 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -1343,6 +1343,9 @@ static struct generic_pm_domain_data
> *genpd_alloc_dev_data(struct device *dev,
> > gpd_data->td.effective_constraint_ns =
> PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
> > gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier;
> >
> > + if (genpd_is_mpd_device(dev))
> > + gpd_data->mpd_data = dev_get_drvdata(dev);
> > +
> > spin_lock_irq(&dev->power.lock);
> >
> > if (dev-&
RE: [PATCH] i2c: imx: correct the method of getting private data in notifier_call
> From: Anson Huang
> Sent: Wednesday, April 17, 2019 10:00 AM
>
> The way of getting private imx_i2c_struct in i2c_imx_clk_notifier_call() is
> incorrect, should use clk_change_nb element to get correct address and avoid
> below kernel dump during POST_RATE_CHANGE notify by clk
> framework:
>
> Unable to handle kernel paging request at virtual address 03ef1488 pgd =
> (ptrval) [03ef1488] *pgd= Internal error: Oops: 5 [#1] PREEMPT SMP
> ARM Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> Workqueue: events reduce_bus_freq_handler PC is at
> i2c_imx_set_clk+0x10/0xb8 LR is at i2c_imx_clk_notifier_call+0x20/0x28
> pc : [<806a893c>]lr : [<806a8a04>]psr: a0080013
> sp : bf399dd8 ip : bf3432ac fp : bf7c1dc0
> r10: 0002 r9 : r8 :
> r7 : 03ef1480 r6 : bf399e50 r5 : r4 :
> r3 : bf025300 r2 : bf399e50 r1 : 00b71b00 r0 : bf399be8
> Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 4e03004a DAC: 0051 Process kworker/2:1
> (pid: 38, stack limit = 0x(ptrval))
> Stack: (0xbf399dd8 to 0xbf39a000)
> 9dc0:
> 806a89e4
> 9de0: bf399e50 0002 806a8a04 806a89e4 80142900
>
> 9e00: bf34ef18 bf34ef04 bf399e50 80142d84
> bf399e6c
> 9e20: bf34ef00 80f214c4 bf025300 0002 80f08d08 bf017480
> 80142df0
> 9e40: 80166ed8 80c27638 8045de58 bf352340 03ef1480
> 00b71b00 0f82e242
> 9e60: bf025300 0002 03ef1480 80f60e5c 0001 8045edf0
> 0002 8045eb08
> 9e80: bf025300 0002 03ef1480 8045ee10 03ef1480 8045eb08
> bf01be40 0002
> 9ea0: 03ef1480 8045ee10 07de2900 8045eb08 bf01b780 0002
> 07de2900 8045ee10
> 9ec0: 80c27898 bf399ee4 bf020a80 0002 1f78a400 8045ee10 80f60e5c
> 80460514
> 9ee0: 80f60e5c bf01b600 bf01b480 80460460 0f82e242 bf383a80 bf383a00
> 80f60e5c
> 9f00: bf7c1dc0 80f60e70 80460564 80f60df0 80f60d24 80f60df0
> 8011e72c
> 9f20: 80f60df0 80f60e6c bf7c4f00 8011e7ac bf274000
> 8013bd84
> 9f40: bf7c1dd8 80f03d00 bf274000 bf7c1dc0 bf274014 bf7c1dd8 80f03d00
> bf398000
> 9f60: 0008 8013bfb4 bf25d100 bf25d0c0
> bf274000 8013bf88
> 9f80: bf25d11c bf0cfebc 8014140c bf25d0c0 801412ec
>
> 9fa0: 801010e8
>
> 9fc0:
>
> 9fe0: 0013
> [<806a893c>] (i2c_imx_set_clk) from [<806a8a04>]
> (i2c_imx_clk_notifier_call+0x20/0x28)
> [<806a8a04>] (i2c_imx_clk_notifier_call) from [<80142900>]
> (notifier_call_chain+0x44/0x84) [<80142900>] (notifier_call_chain) from
> [<80142d84>] (__srcu_notifier_call_chain+0x44/0x98)
> [<80142d84>] (__srcu_notifier_call_chain) from [<80142df0>]
> (srcu_notifier_call_chain+0x18/0x20)
> [<80142df0>] (srcu_notifier_call_chain) from [<8045de58>]
> (__clk_notify+0x78/0xa4) [<8045de58>] (__clk_notify) from [<8045edf0>]
> (__clk_recalc_rates+0x60/0xb4) [<8045edf0>] (__clk_recalc_rates) from
> [<8045ee10>] (__clk_recalc_rates+0x80/0xb4)
> Code: e92d40f8 e5903298 e59072a0 e1530001 (e5975008) ---[ end trace
> fc7f5514b97b6cbb ]---
>
> Fixes: 90ad2cbe88c2("i2c: imx: use clk notifier for rate changes")
> Signed-off-by: Anson Huang
Please also provide how to reproduce it.
And it seems not a new issue, should we CC stable?
Regards
Dong Aisheng
> ---
> drivers/i2c/busses/i2c-imx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index
> c0c3043..fd70b11 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -515,9 +515,9 @@ static int i2c_imx_clk_notifier_call(struct
> notifier_block *nb,
>unsigned long action, void *data) {
> struct clk_notifier_data *ndata = data;
> - struct imx_i2c_struct *i2c_imx = container_of(&ndata->clk,
> + struct imx_i2c_struct *i2c_imx = container_of(nb,
> struct imx_i2c_struct,
> - clk);
> + clk_change_nb);
>
> if (action & POST_RATE_CHANGE)
> i2c_imx_set_clk(i2c_imx, ndata->new_rate);
> --
> 2.7.4
RE: [PATCH] i2c: imx: correct the method of getting private data in notifier_call
[...]
> >
> > Fixes: 90ad2cbe88c2("i2c: imx: use clk notifier for rate changes")
> > Signed-off-by: Anson Huang
>
> Please also provide how to reproduce it.
> And it seems not a new issue, should we CC stable?
Besides above comments:
Reviewed-by: Dong Aisheng
Regards
Dong Aisheng
>
> Regards
> Dong Aisheng
>
> > ---
> > drivers/i2c/busses/i2c-imx.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx.c
> > b/drivers/i2c/busses/i2c-imx.c index
> > c0c3043..fd70b11 100644
> > --- a/drivers/i2c/busses/i2c-imx.c
> > +++ b/drivers/i2c/busses/i2c-imx.c
> > @@ -515,9 +515,9 @@ static int i2c_imx_clk_notifier_call(struct
> > notifier_block *nb,
> > unsigned long action, void *data) {
> > struct clk_notifier_data *ndata = data;
> > - struct imx_i2c_struct *i2c_imx = container_of(&ndata->clk,
> > + struct imx_i2c_struct *i2c_imx = container_of(nb,
> > struct imx_i2c_struct,
> > - clk);
> > + clk_change_nb);
> >
> > if (action & POST_RATE_CHANGE)
> > i2c_imx_set_clk(i2c_imx, ndata->new_rate);
> > --
> > 2.7.4
RE: [PATCH V12 1/5] dt-bindings: fsl: scu: add thermal binding
> From: Anson Huang > Sent: Tuesday, April 16, 2019 11:22 AM > > NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system > controller, the system controller is in charge of system power, clock and > thermal sensors etc. management, Linux kernel has to communicate with > system controller via MU (message unit) IPC to get temperature from thermal > sensors, this patch adds binding doc for i.MX system controller thermal > driver. > > Signed-off-by: Anson Huang > --- > No changes. > --- > .../devicetree/bindings/arm/freescale/fsl,scu.txt| 16 > > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt > index 5d7dbab..f4fb6d5 100644 > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt > @@ -133,6 +133,17 @@ RTC bindings based on SCU Message Protocol > Required properties: > - compatible: should be "fsl,imx8qxp-sc-rtc"; > > +Thermal bindings based on SCU Message Protocol > + > + > +Required properties: > +- compatible:Should be : > + "fsl,imx8qxp-sc-thermal" > + followed by "fsl,imx-sc-thermal"; > + > +- #thermal-sensor-cells: See > Documentation/devicetree/bindings/thermal/thermal.txt > + for a description. Better to have an explicit value here. e.g. Must be 1. See xxx for a description. Otherwise: Reviewed-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH V12 3/5] thermal: imx_sc: add i.MX system controller thermal support
> From: Anson Huang
> Sent: Tuesday, April 16, 2019 11:22 AM
>
> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller inside,
> the system controller is in charge of controlling power, clock and thermal
> sensors etc..
>
> This patch adds i.MX system controller thermal driver support, Linux kernel
> has
> to communicate with system controller via MU (message unit) IPC to get each
> thermal sensor's temperature, it supports multiple sensors which are passed
> from device tree, please see the binding doc for details.
>
> Signed-off-by: Anson Huang
> ---
> Changes since V11:
> - move the API of getting thermal zone sensor ID to of-thermal.c as
> generic API;
> - remove unnecessary __packed.
> ---
> drivers/thermal/Kconfig | 11
> drivers/thermal/Makefile | 1 +
> drivers/thermal/imx_sc_thermal.c | 137
> +++
> 3 files changed, 149 insertions(+)
> create mode 100644 drivers/thermal/imx_sc_thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> 653aa27..4e4fa7e 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -223,6 +223,17 @@ config IMX_THERMAL
> cpufreq is used as the cooling device to throttle CPUs when the
> passive trip is crossed.
>
> +config IMX_SC_THERMAL
> + tristate "Temperature sensor driver for NXP i.MX SoCs with System
> Controller"
> + depends on (ARCH_MXC && IMX_SCU) || COMPILE_TEST
COMPILE_TEST seems won't work and may cause build issue due to
the dependency of IMX_SCU.
> + depends on OF
> + help
> + Support for Temperature Monitor (TEMPMON) found on NXP i.MX SoCs
> with
> + system controller inside, Linux kernel has to communicate with system
> + controller via MU (message unit) IPC to get temperature from thermal
> + sensor. It supports one critical trip point and one
> + passive trip point for each thermal sensor.
> +
[...]
> +static int imx_sc_thermal_get_temp(void *data, int *temp) {
> + struct imx_sc_msg_misc_get_temp msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + struct imx_sc_sensor *sensor = data;
> + int ret;
> +
> + msg.data.req.resource_id = sensor->resource_id;
> + msg.data.req.type = IMX_SC_C_TEMP;
IMX_SC_C_TEMP duplicates with enum imx_sc_ctrl we already have.
include/linux/firmware/imx/types.h
But I guess you're not want to use a CTRL here.
So please fix the name convention.
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_MISC;
> + hdr->func = IMX_SC_MISC_FUNC_GET_TEMP;
> + hdr->size = 2;
> +
> + ret = imx_scu_call_rpc(thermal_ipc_handle, &msg, true);
> + if (ret) {
> + pr_err("read temp sensor %d failed, ret %d\n",
> + sensor->resource_id, ret);
The error message is meaningless.
Can't convert to dev_err?
> + return ret;
> + }
> +
> + *temp = msg.data.resp.celsius * 1000 + msg.data.resp.tenths * 100;
> +
> + return 0;
> +}
> +
> +static const struct thermal_zone_of_device_ops imx_sc_thermal_ops = {
> + .get_temp = imx_sc_thermal_get_temp,
> +};
> +
> +static int imx_sc_thermal_probe(struct platform_device *pdev) {
> + struct device_node *np, *child;
> + int ret;
> +
> + ret = imx_scu_get_handle(&thermal_ipc_handle);
> + if (ret)
> + return ret;
> +
> + np = of_find_node_by_name(NULL, "thermal-zones");
> + if (!np)
> + return -ENODEV;
> +
> + for_each_available_child_of_node(np, child) {
> + struct of_phandle_args sensor_specs;
> + struct imx_sc_sensor *sensor =
> + devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
Usually we do not mix the complicated code in the declare line.
You can move the declarations into the top level to make code more clean.
> + if (!sensor)
> + return -ENOMEM;
> +
> + ret = thermal_zone_of_get_sensor_id(child,
> + &sensor_specs,
It looks a bit strange why need sensor_specs as you even did not use it.
Regards
Dong Aisheng
> + &sensor->resource_id);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "failed to get valid sensor resource id: %d\n",
> + ret);
> + break;
> + }
> +
> + sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> +
> sensor->resource_id,
> +sensor,
> +
> &imx_sc_thermal_ops);
> + if (IS_ERR(sensor->tzd)) {
> + dev_err(&pdev->dev, "failed to register thermal
> zone\n");
> +
RE: [RFC PATCH 0/3] Add support of busfreq
+Jacky and Leonard, Ranjani Hi Alexandre, > From: Alexandre Bailon [mailto:abai...@baylibre.com] > > This series implements busfreq, a framework used in MXP's tree to scale the > interconnect and dram frequencies. > In the vendor tree, device's driver request for a performance level, which is > used to scale the frequencies. > This series implements it using the interconnect framework. > Devices' driver request for bandwidth which is use by busfreq to determine a > performance level, and then scale the frequency. > > Busfreq is quite generic. It could be used for any i.MX SoC. > A busfreq platform driver just have to define a list of interconnect nodes, > and > some OPPs. > It's really great to see this patch series. And it should be the correct direction we're heading to upstream busfreq support. > This series is sent as RFC mostly because the current support of i.MX SoC > won't > benefit of busfreq framework, because the clocks' driver don't support > interconnect / dram frequency scaling. > As exemple, this series implements busfreq for i.MX8MM whose upstreaming > is in progress. Because this relies on ATF to do the frequency scaling, it > won't > be hard make it work. > How can I test this patch series? Any additional patches you can share with us? Or what else we need to do to test it? We can help with it. Regards Dong Aisheng > As exemple, this series implements busfreq for Alexandre Bailon (3): > drivers: interconnect: Add a driver for i.MX SoC > drivers: interconnect: imx: Add support of i.MX8MM > dt-bindings: interconnect: Document fsl,busfreq-imx8mm bindings > > .../bindings/interconnect/imx8mm.txt | 24 + > drivers/interconnect/Kconfig | 1 + > drivers/interconnect/Makefile | 1 + > drivers/interconnect/imx/Kconfig | 17 + > drivers/interconnect/imx/Makefile | 2 + > drivers/interconnect/imx/busfreq-imx8mm.c | 132 > drivers/interconnect/imx/busfreq.c| 570 > ++ > drivers/interconnect/imx/busfreq.h| 123 > include/dt-bindings/interconnect/imx8mm.h | 37 ++ > 9 files changed, 907 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/interconnect/imx8mm.txt > create mode 100644 drivers/interconnect/imx/Kconfig create mode > 100644 drivers/interconnect/imx/Makefile create mode 100644 > drivers/interconnect/imx/busfreq-imx8mm.c > create mode 100644 drivers/interconnect/imx/busfreq.c > create mode 100644 drivers/interconnect/imx/busfreq.h > create mode 100644 include/dt-bindings/interconnect/imx8mm.h > > -- > 2.19.2
RE: [PATCH 1/2] ARM: imx: drop uneccessary of_platform_default_populate
> From: Peng Fan
>
> "arch_initcall_sync(of_platform_default_populate_init);" could be used to
> populate the device tree, there is no need to call
> of_platform_default_populate
> in machine code.
>
> Tested on i.MX6Q-SDB i.MX6SL-EVK i.MX6UL-EVK board.
>
> Signed-off-by: Peng Fan
> ---
> arch/arm/mach-imx/mach-imx6q.c | 2 --
> arch/arm/mach-imx/mach-imx6sl.c | 2 --
> arch/arm/mach-imx/mach-imx6sx.c | 2 --
> arch/arm/mach-imx/mach-imx6ul.c | 1 -
> 4 files changed, 7 deletions(-)
>
> diff --git a/arch/arm/mach-imx/mach-imx6q.c
> b/arch/arm/mach-imx/mach-imx6q.c index 7d80a0ae723c..655398c20256
> 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -278,8 +278,6 @@ static void __init imx6q_init_machine(void)
>
> imx6q_enet_phy_init();
>
> - of_platform_default_populate(NULL, NULL, parent);
> -
This slightly change the device register sequence as well as possible driver
probe
sequence as a consequence.
Originally devices are registered in arch_initcall. Now it will be a bit later
in arch_initcall_sync and this may cause a bit risk if the code under the
default_populate want to access the device service provided by early probe.
Probably it's more safe to leave as it is unless you can double confirm
there're no such code depends on accessing early probed devices as follows
before we can make the change.
> imx_anatop_init();
> cpu_is_imx6q() ? imx6q_pm_init() : imx6dl_pm_init();
> imx6q_1588_init();
Regards
Dong Aisheng
> diff --git a/arch/arm/mach-imx/mach-imx6sl.c
> b/arch/arm/mach-imx/mach-imx6sl.c index 99be4225297a..9743bdbb68fa
> 100644
> --- a/arch/arm/mach-imx/mach-imx6sl.c
> +++ b/arch/arm/mach-imx/mach-imx6sl.c
> @@ -56,8 +56,6 @@ static void __init imx6sl_init_machine(void)
> if (parent == NULL)
> pr_warn("failed to initialize soc device\n");
>
> - of_platform_default_populate(NULL, NULL, parent);
> -
> if (cpu_is_imx6sl())
> imx6sl_fec_init();
> imx_anatop_init();
> diff --git a/arch/arm/mach-imx/mach-imx6sx.c
> b/arch/arm/mach-imx/mach-imx6sx.c index 7f52d9b1e8a4..19b9f2dd309e
> 100644
> --- a/arch/arm/mach-imx/mach-imx6sx.c
> +++ b/arch/arm/mach-imx/mach-imx6sx.c
> @@ -72,8 +72,6 @@ static void __init imx6sx_init_machine(void)
> if (parent == NULL)
> pr_warn("failed to initialize soc device\n");
>
> - of_platform_default_populate(NULL, NULL, parent);
> -
> imx6sx_enet_init();
> imx_anatop_init();
> imx6sx_pm_init();
> diff --git a/arch/arm/mach-imx/mach-imx6ul.c
> b/arch/arm/mach-imx/mach-imx6ul.c index 6cb8a22b617d..c57b9df791b1
> 100644
> --- a/arch/arm/mach-imx/mach-imx6ul.c
> +++ b/arch/arm/mach-imx/mach-imx6ul.c
> @@ -65,7 +65,6 @@ static void __init imx6ul_init_machine(void)
> if (parent == NULL)
> pr_warn("failed to initialize soc device\n");
>
> - of_platform_default_populate(NULL, NULL, parent);
> imx6ul_enet_init();
> imx_anatop_init();
> imx6ul_pm_init();
> --
> 2.16.4
RE: [PATCH 2/2] ARM: imx: mach-imx7ulp: warn when imx_soc_device_init fail
> From: Peng Fan
>
> [PATCH 2/2] ARM: imx: mach-imx7ulp: warn when imx_soc_device_init fail
ARM: imx: imx7ulp: ...
> Follow other i.MX6/7 machince code to check return value of
> imx_soc_device_init and warn when fail.
>
> Also drop of_platform_default_populate, because
> "arch_initcall_sync(of_platform_default_populate_init);" could be used to
> populate the device tree.
>
This could be in separate patch.
> Signed-off-by: Peng Fan
> ---
> arch/arm/mach-imx/mach-imx7ulp.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-imx/mach-imx7ulp.c
> b/arch/arm/mach-imx/mach-imx7ulp.c
> index 11ac71aaf965..6d823f05d9aa 100644
> --- a/arch/arm/mach-imx/mach-imx7ulp.c
> +++ b/arch/arm/mach-imx/mach-imx7ulp.c
> @@ -53,11 +53,16 @@ static void __init imx7ulp_set_revision(void)
>
> static void __init imx7ulp_init_machine(void) {
> + struct device *soc_dev;
> +
> + soc_dev = imx_soc_device_init();
> + if (soc_dev == NULL)
> + pr_warn("failed to initialize soc device\n");
> +
Should this be under set revision?
Regards
Dong Aisheng
> imx7ulp_pm_init();
>
> mxc_set_cpu_type(MXC_CPU_IMX7ULP);
> imx7ulp_set_revision();
> - of_platform_default_populate(NULL, NULL, imx_soc_device_init());
> }
>
> static const char *const imx7ulp_dt_compat[] __initconst = {
> --
> 2.16.4
RE: [PATCH] arm64: dts: imx8qxp: add lsio_mu2 node
> From: Peng Fan
>
> Add lsio_mu2 node which could be used communicate with SCU.
>
> Signed-off-by: Peng Fan
Reviewed-by: Dong Aisheng
Regards
Dong Aisheng
> ---
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 16766838ac77..64eecd6cb1e9 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -339,6 +339,14 @@
> #mbox-cells = <2>;
> };
>
> + lsio_mu2: mailbox@5d1d {
> + compatible = "fsl,imx8qxp-mu", "fsl,imx6sx-mu";
> + reg = <0x5d1d 0x1>;
> + interrupts = ;
> + #mbox-cells = <2>;
> + status = "disabled";
> + };
> +
> lsio_mu3: mailbox@5d1e {
> compatible = "fsl,imx8qxp-mu", "fsl,imx6sx-mu";
> reg = <0x5d1e 0x1>;
> --
> 2.16.4
RE: [PATCH] arm64: dts: imx8qxp: fix mbox-cells
> From: Peng Fan > > Subject: [PATCH] arm64: dts: imx8qxp: fix mbox-cells > > Currently lsio_mu1 is used by Linux Kernel with mbox-cells as 2, but actually > mu0-4 could be used to communicate with SCU. So fix the mbox-cells. > > Signed-off-by: Peng Fan Reviewed-by: Dong Aisheng Regards Dong Aisheng > --- > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > index 4c3dd95ed488..16766838ac77 100644 > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > @@ -328,7 +328,7 @@ > compatible = "fsl,imx8qxp-mu", "fsl,imx6sx-mu"; > reg = <0x5d1b 0x1>; > interrupts = ; > - #mbox-cells = <0>; > + #mbox-cells = <2>; > status = "disabled"; > }; > > @@ -343,7 +343,7 @@ > compatible = "fsl,imx8qxp-mu", "fsl,imx6sx-mu"; > reg = <0x5d1e 0x1>; > interrupts = ; > - #mbox-cells = <0>; > + #mbox-cells = <2>; > status = "disabled"; > }; > > @@ -351,7 +351,7 @@ > compatible = "fsl,imx8qxp-mu", "fsl,imx6sx-mu"; > reg = <0x5d1f 0x1>; > interrupts = ; > - #mbox-cells = <0>; > + #mbox-cells = <2>; > status = "disabled"; > }; > > -- > 2.16.4
RE: [PATCH V4 1/4] dt-bindings: fsl: scu: add general interrupt support
> From: Anson Huang > > Add scu general interrupt function support. > > Signed-off-by: Anson Huang > Reviewed-by: Rob Herring Reviewed-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH V4 2/4] firmware: imx: enable imx scu general irq function
> From: Anson Huang
>
> The System Controller Firmware (SCFW) controls RTC, thermal and WDOG etc.,
> these resources' interrupt function are managed by SCU. When any IRQ
> pending, SCU will notify Linux via MU general interrupt channel #3, and Linux
> kernel needs to call SCU APIs to get IRQ status and notify each module to
> handle the interrupt.
>
> Since there is no data transmission for SCU IRQ notification, so doorbell mode
> is used for this MU channel, and SCU driver will use notifier mechanism to
> broadcast to every module which registers the SCU block notifier.
>
> Signed-off-by: Anson Huang
> ---
> Changes since V3:
> - use alias to get general MU interrupt channel id and then get resource
> ID,
> this is to support different MU instance;
> - add return value check for imx_scu_enable_general_irq_channel().
> ---
> drivers/firmware/imx/imx-scu.c | 116
> +++
Generally I would suggest to put scu irq support into another separate file
under
The same folder to make code clean from function point of view.
> include/linux/firmware/imx/sci.h | 3 +
> 2 files changed, 119 insertions(+)
>
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index 2bb1a19..1dcd7b3 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -7,6 +7,7 @@
> *
> */
>
> +#include
> #include
> #include
> #include
> @@ -21,6 +22,8 @@
>
> #define SCU_MU_CHAN_NUM 8
> #define MAX_RX_TIMEOUT (msecs_to_jiffies(30))
> +#define IMX_SC_IRQ_FUNC_STATUS 2
> +#define IMX_SC_IRQ_NUM_GROUP 6
>
> struct imx_sc_chan {
> struct imx_sc_ipc *sc_ipc;
> @@ -41,6 +44,7 @@ struct imx_sc_ipc {
> u32 *msg;
> u8 rx_size;
> u8 count;
> + u32 mu_resource_id;
I feel it a bit strange to put this mu id in struct imx_sc_ipc.
> };
>
> /*
> @@ -77,7 +81,23 @@ static int imx_sc_linux_errmap[IMX_SC_ERR_LAST] = {
> -EIO,/* IMX_SC_ERR_FAIL */
> };
>
> +struct imx_sc_msg_irq_get_status {
> + struct imx_sc_rpc_msg hdr;
> + union {
> + struct {
> + u16 resource;
> + u8 group;
> + u8 reserved;
> + } __packed req;
> + struct {
> + u32 status;
> + } __packed resp;
No packed needed for this one
> + } data;
> +};
> +
> static struct imx_sc_ipc *imx_sc_ipc_handle;
> +static struct work_struct imx_sc_general_irq_work; static
> +BLOCKING_NOTIFIER_HEAD(imx_scu_notifier_chain);
Imx_scu_irq_xxx
>
> static inline int imx_sc_to_linux_errno(int errno) { @@ -194,9 +214,90 @@
> int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void *msg, bool have_resp) }
> EXPORT_SYMBOL(imx_scu_call_rpc);
>
> +int imx_scu_register_notifier(struct notifier_block *nb) {
> + return blocking_notifier_chain_register(&imx_scu_notifier_chain, nb);
> +} EXPORT_SYMBOL(imx_scu_register_notifier);
> +
> +int imx_scu_unregister_notifier(struct notifier_block *nb) {
> + return blocking_notifier_chain_unregister(&imx_scu_notifier_chain,
> +nb); } EXPORT_SYMBOL(imx_scu_unregister_notifier);
> +
> +static int imx_scu_notifier_call_chain(unsigned long status, u8 *group)
> +{
> + return blocking_notifier_call_chain(&imx_scu_notifier_chain,
> + status, (void *)group);
> +}
> +
> +static void imx_scu_general_irq_work_handler(struct work_struct *work)
> +{
> + struct imx_sc_msg_irq_get_status msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + u32 irq_status;
> + int ret;
> + u8 i;
> +
> + for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
Do we need to support all irq group?
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_IRQ;
> + hdr->func = IMX_SC_IRQ_FUNC_STATUS;
> + hdr->size = 2;
> +
> + msg.data.req.resource = imx_sc_ipc_handle->mu_resource_id;
> + msg.data.req.group = i;
> +
> + ret = imx_scu_call_rpc(imx_sc_ipc_handle, &msg, true);
> + if (ret) {
> + pr_err("get irq status failed, ret %d\n", ret);
Can the error output more useful information?
> + return;
> + }
> +
> + irq_status = msg.data.resp.status;
> + if (!irq_status)
> + continue;
> +
> + imx_scu_notifier_call_chain(irq_status, &i);
> + }
> +}
> +
> +static void imx_scu_rxdb_callback(struct mbox_client *c, void *msg) {
Imx_scu_irq_callback
> + schedule_work(&imx_sc_general_irq_work);
> +}
> +
> +static int imx_scu_enable_general_irq_channel(struct device *dev) {
> + struct mbox_client *cl;
> + struct mbox_chan *ch;
> + int ret = 0;
> +
> + cl = devm_kzalloc(dev, sizeof(*cl), GFP_KERNEL);
> + if (!cl)
> + return -ENOMEM;
> +
> + cl->dev = dev;
> + cl->rx_
RE: [PATCH V4 3/4] arm64: dts: freescale: imx8qxp: enable scu general irq channel
> From: Anson Huang > > On i.MX8QXP, SCU uses MU1 general interrupt channel #3 to notify user for > IRQs of RTC alarm, thermal alarm and WDOG etc., mailbox RX doorbell mode is > used for this function, this patch adds support for it. > > Signed-off-by: Anson Huang > --- > Changes since V3: > - rename "gi3" to "gip3"; > - add alias for getting mu id by driver. > --- > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > index 4c3dd95..f0a9224 100644 > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi > @@ -21,6 +21,7 @@ > mmc1 = &usdhc2; > mmc2 = &usdhc3; > serial0 = &adma_lpuart0; > + mu1 = &lsio_mu1; Need add aliase in binding doc. Otherwise: Reviewed-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH] dt-bindings: clock: imx8mq: Fix numbering overlaps and gaps
> From: Stephen Boyd [mailto:sb...@kernel.org] > > Quoting Patrick Wildt (2019-03-12 00:36:54) > > On Fri, Mar 08, 2019 at 07:29:05AM -0800, Stephen Boyd wrote: > > > It's mostly about making sure that any existing dtbs don't have > > > their numbers shifted around. So hopefully any overlapping > > > identifiers aren't in use yet and then those ids can be changed > > > while leaving the ones that are in use how they are. > > > > In practice I bet no one uses Linux 5.0's i.MX8M device trees since > > they lack too much support. It's so basic it's not useful. You'd > > still run your existing non-mainline bindings until it is. Thus I > > would argue changing the ABI right now would be the only chance there is. > > > > If you think that chance is gone, then I guess the reasonable thing is > > to keep the numbers and only move those (to the end) which overlap. > > Or put them into that erreneous number gap. > > > > The chance is quickly slipping away because we're going to be at -rc1 soon. > I'm > not the one to decide what is and isn't being used by people out there, so I'm > happy to apply this patch now before the next -rc1 comes out as long as it > doesn't break anything in arm-soc area. The confidence I'm getting isn't high > though. Has anyone from NXP reviewed this change? Maybe I can get an ack > from someone else that normally looks after the arm-soc/dts side of things > here indicating that nothing should go wrong? That would increase my > confidence levels. AFAIK no one out there using it for product without being able to update accordingly, as it still has a very preliminary support. So I agree we need to fix it at this early time Tested-and-Acked-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH 1/2] ARM: imx: drop uneccessary of_platform_default_populate
[...] > > Originally devices are registered in arch_initcall. Now it will be a > > bit later in arch_initcall_sync and this may cause a bit risk if the > > code under the default_populate want to access the device service provided > by early probe. > > > > Probably it's more safe to leave as it is unless you can double > > confirm there're no such code depends on accessing early probed > > devices as follows before we can make the change. > > This has been boot tested on 6Q-SDB/6UL-EVK/6SL-EVK board. > For i.MX, I only see pinctrl driver use arch_initcall from the link, > https://elixir.bootlin.com/linux/latest/ident/arch_initcall > > From my boot test, the pinctrl driver probe will be delayed a bit, but I do > not > see issues. > >From what I saw, imx6q_1588_init() and imx6q_axi_init() will use syscon which >is registered with postcore_initcall. Without having syscon devices populated, I wonder those calls may fall. Can you double check it? Regards Dong Aisheng
RE: [PATCH 2/2] ARM: imx: mach-imx7ulp: warn when imx_soc_device_init fail
[...]
> > > Follow other i.MX6/7 machince code to check return value of
> > > imx_soc_device_init and warn when fail.
> > >
> > > Also drop of_platform_default_populate, because
> > > "arch_initcall_sync(of_platform_default_populate_init);" could be
> > > used to populate the device tree.
> > >
> >
> > This could be in separate patch.
>
> I'll do it in v2 after we agree the change in the first patch.
>
I think imx7ulp does not have the issue in patch 1.
Regards
Dong Aisheng
> >
> > > Signed-off-by: Peng Fan
> > > ---
> > > arch/arm/mach-imx/mach-imx7ulp.c | 7 ++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-imx/mach-imx7ulp.c
> > > b/arch/arm/mach-imx/mach-imx7ulp.c
> > > index 11ac71aaf965..6d823f05d9aa 100644
> > > --- a/arch/arm/mach-imx/mach-imx7ulp.c
> > > +++ b/arch/arm/mach-imx/mach-imx7ulp.c
> > > @@ -53,11 +53,16 @@ static void __init imx7ulp_set_revision(void)
> > >
> > > static void __init imx7ulp_init_machine(void) {
> > > + struct device *soc_dev;
> > > +
> > > + soc_dev = imx_soc_device_init();
> > > + if (soc_dev == NULL)
> > > + pr_warn("failed to initialize soc device\n");
> > > +
> >
> > Should this be under set revision?
>
> Just follow other i.MX6/7 platforms practice, I could move this under set
> revision in v2 if you prefer.
>
> Thanks,
> Peng.
>
> >
> > Regards
> > Dong Aisheng
> >
> > > imx7ulp_pm_init();
> > >
> > > mxc_set_cpu_type(MXC_CPU_IMX7ULP);
> > > imx7ulp_set_revision();
> > > - of_platform_default_populate(NULL, NULL, imx_soc_device_init());
> > > }
> > >
> > > static const char *const imx7ulp_dt_compat[] __initconst = {
> > > --
> > > 2.16.4
RE: [PATCH 2/2] arm64: dts: imx8qxp: Add EDMA0/EDMA1 nodes
> From: Daniel Baluta
> Sent: Tuesday, March 26, 2019 5:43 PM
>
> i.MX8QXP contains a total of 4 EDMA controllers of which two are primarily
> for audio components and the other two are for non-audio periperhals.
>
> This patch adds the EDMA0/EDMA1 nodes used by audio peripherals.
>
> EDMA0 contains channels for:
> * ASRC0
> * ESAI0
> * SPDIF0
> * SAI0, SAI1, SAI2, SAI3
>
> EDMA1 contains channels for:
> * ASRC1
> * SAI4, SAI5
>
> See chapter Audio DMA Memory Maps (2.2.3) from i.MX8QXP RM [1]
>
> This patch is based on the dtsi file initially submitted by Teo Hall in i.MX
> NXP
> internal tree.
>
> [1] https://www.nxp.com/docs/en/reference-manual/IMX8DQXPRM.pdf
>
> Cc: Teo Hall
> Signed-off-by: Daniel Baluta
> ---
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 72
> ++
> 1 file changed, 72 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 0cb939861a60..9e959deb2499 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -182,6 +182,78 @@
> #clock-cells = <1>;
> };
>
> + edma0: dma-controller@591f {
> + compatible = "fsl,imx8qm-edma";
Should this be "fsl,imx8qxp-edma"?
or
"fsl,imx8qxp-edma", "fsl,imx8qm-edma"?
Regards
Dong Aisheng
> + reg = <0x5920 0x1>, /* asrc0 pair A input req
> */
> + <0x5921 0x1>, /* asrc0 pair B input req
> */
> + <0x5922 0x1>, /* asrc0 pair C input req
> */
> + <0x5923 0x1>, /* asrc0 pair A output
> req */
> + <0x5924 0x1>, /* asrc0 pair B output
> req */
> + <0x5925 0x1>, /* asrc0 pair C output
> req */
> + <0x5926 0x1>, /* esai0 rx */
> + <0x5927 0x1>, /* esai0 tx */
> + <0x5928 0x1>, /* spdif0 rx */
> + <0x5929 0x1>, /* spdif0 tx */
> + <0x592c 0x1>, /* sai0 rx */
> + <0x592d 0x1>, /* sai0 tx */
> + <0x592e 0x1>, /* sai1 rx */
> + <0x592f 0x1>, /* sai1 tx */
> + #dma-cells = <3>;
> + shared-interrupt;
> + dma-channels = <14>;
> + interrupts = ,/*
> asrc 0
> */
> + ,
> + ,
> + ,
> + ,
> + ,
> + , /*
> esai0 */
> + ,
> + , /*
> spdif0 */
> + ,
> + , /*
> sai0 */
> + ,
> + , /*
> sai1 */
> + ,
> + interrupt-names = "edma0-chan0-rx", "edma0-chan1-rx", /*
> asrc0 */
> + "edma0-chan2-rx", "edma0-chan3-tx",
> + "edma0-chan4-tx", "edma0-chan5-tx",
> + "edma0-chan6-rx", "edma0-chan7-tx",
> /* esai0 */
> + "edma0-chan8-rx", "edma0-chan9-tx",
> /* spdif0 */
> + "edma0-chan12-rx", "edma0-chan13-tx",
> /* sai0 */
> + "edma0-chan14-rx", "edma0-chan15-tx",
> /* sai1 */
> + };
> +
> + edma1: dma-controller@599F {
> + compatible = "fsl,imx8qm-edma";
> + reg = <0x0 0x59A0 0x0 0x1>, /* asrc1 */
> + <0x0 0x59A1 0x0 0x1>,
> + <0x0 0x59A2 0x0 0x1>,
> + <0x0 0x59A3 0x0 0x1>,
> + <0x0 0x59A4 0x0 0x1>,
> + <0x0 0x59A5 0x0 0x1>,
> + <0x0 0x59A8 0x0 0x1>, /* sai4 rx */
> + <0x0 0x59A9 0x0 0x1>, /* sai4 tx */
> + <0x0 0x59AA 0x0 0x1>; /* sai5 tx */
> + #dma-cells = <3>;
> + shared-interrupt;
> + dma-channels = <9>;
> + interrupts = , /* asrc
> 1 */
> + ,
> + ,
> + ,
> + ,
>
RE: [PATCH v2 2/2] arm64: dts: imx8qxp: Add EDMA0/EDMA1 nodes
> From: Daniel Baluta
> Sent: Thursday, March 28, 2019 3:03 AM
>
> i.MX8QXP contains a total of 4 EDMA controllers of which two are primarily
> for audio components and the other two are for non-audio periperhals.
>
> This patch adds the EDMA0/EDMA1 nodes used by audio peripherals.
>
> EDMA0 contains channels for:
> * ASRC0
> * ESAI0
> * SPDIF0
> * SAI0, SAI1, SAI2, SAI3
>
> EDMA1 contains channels for:
> * ASRC1
> * SAI4, SAI5
>
> See chapter Audio DMA Memory Maps (2.2.3) from i.MX8QXP RM [1]
>
> This patch is based on the dtsi file initially submitted by Teo Hall in i.MX
> NXP
> internal tree.
>
> [1] https://www.nxp.com/docs/en/reference-manual/IMX8DQXPRM.pdf
>
> Cc: Teo Hall
> Signed-off-by: Daniel Baluta
> ---
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 72
> ++
> 1 file changed, 72 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 0cb939861a60..84c7c3eca1a1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -182,6 +182,78 @@
> #clock-cells = <1>;
> };
>
> + edma0: dma-controller@591f {
> + compatible = "fsl,imx8qxp-edma";
> + reg = <0x5920 0x1>, /* asrc0 pair A input req
> */
> + <0x5921 0x1>, /* asrc0 pair B input req
> */
> + <0x5922 0x1>, /* asrc0 pair C input req
> */
> + <0x5923 0x1>, /* asrc0 pair A output
> req */
> + <0x5924 0x1>, /* asrc0 pair B output
> req */
> + <0x5925 0x1>, /* asrc0 pair C output
> req */
> + <0x5926 0x1>, /* esai0 rx */
> + <0x5927 0x1>, /* esai0 tx */
> + <0x5928 0x1>, /* spdif0 rx */
> + <0x5929 0x1>, /* spdif0 tx */
> + <0x592c 0x1>, /* sai0 rx */
> + <0x592d 0x1>, /* sai0 tx */
> + <0x592e 0x1>, /* sai1 rx */
> + <0x592f 0x1>, /* sai1 tx */
> + #dma-cells = <3>;
In binding doc, it's 2.
- #dma-cells : Must be <2>.
The 1st cell specifies the DMAMUX(0 for DMAMUX0 and 1 for DMAMUX1).
Specific request source can only be multiplexed by specific channels
group called DMAMUX.
The 2nd cell specifies the request source(slot) ID.
Need update binding doc?
> + shared-interrupt;
Undocumented property?
Checkpatch did not complain?
Regards
Dong Aisheng
> + dma-channels = <14>;
> + interrupts = ,/*
> asrc 0
> */
> + ,
> + ,
> + ,
> + ,
> + ,
> + , /*
> esai0 */
> + ,
> + , /*
> spdif0 */
> + ,
> + , /*
> sai0 */
> + ,
> + , /*
> sai1 */
> + ,
> + interrupt-names = "edma0-chan0-rx", "edma0-chan1-rx", /*
> asrc0 */
> + "edma0-chan2-rx", "edma0-chan3-tx",
> + "edma0-chan4-tx", "edma0-chan5-tx",
> + "edma0-chan6-rx", "edma0-chan7-tx",
> /* esai0 */
> + "edma0-chan8-rx", "edma0-chan9-tx",
> /* spdif0 */
> + "edma0-chan12-rx", "edma0-chan13-tx",
> /* sai0 */
> + "edma0-chan14-rx", "edma0-chan15-tx",
> /* sai1 */
> + };
> +
> + edma1: dma-controller@599F {
> + compatible = "fsl,imx8qxp-edma";
> + reg = <0x0 0x59A0 0x0 0x1>, /* asrc1 */
> + <0x0 0x59A1 0x0 0x1>,
> + <0x0 0x59A2 0x0 0x1>,
> + <0x0 0x59A3 0x0 0x1>,
> + <0x0 0x59A4 0x0 0x1>,
> + <0x0 0x59A5 0x0 0x1>,
> + <0x0 0x59A8 0x0 0x1>, /* sai4 rx */
> + <0x0 0x59A9 0x0 0x1>, /* sai4 tx */
> + <0x0 0x59AA 0x0 0x1>; /* sai5 tx */
> + #dma-cells = <3>;
> +
RE: Issues with i.MX SPI DMA transfers
> From: Igor Plyatov [mailto:plya...@gmail.com]
>
> Dear developers,
>
> please, help to resolve two issues with SPI DMA transfers at i.MX6Q platform.
>
> First issue is
> [ 4465.008003] spi_master spi0: I/O Error in DMA RX
>
> Second issue is duplication for one of received bytes.
>
Copy Xu Han, Clark, and Yibin who may help
Regards
Dong Aisheng
> Probably, these issues related to each one.
>
> I think suspicious code is one of files linux/drivers/spi/spi-imx.c,
> linux/drivers/dma/imx-sdma.c, /lib/firmware/imx/sdma/sdma-imx6q.bin.
>
>
> My environment
> --
>
> Linux kernel: 5.1.0-rc2 from Linus mainline master branch, where last
> commit is 14c741de93861749dfb60b4964028541f5c506ca from Tue Mar 26
> 14:25:48 2019 -0700.
>
> File "drivers/dma/imx-sdma.c" has been patched by me, because I have
> kernel errors "imx-sdma 20ec000.sdma: Timeout waiting for CH0 ready".
> Where patch was taken from email of Andy Duan with Subject "[PATCH v2
> dmaengine 1/1] dmaengine: imx-sdma: revert: add clock ratio 1:1 check"
> E-mail can be found at
> ...
>
> Issues exists in Linux kernel 4.9 too.
>
> SOM (System On Module) is from Toradex and called "Apalis iMX6 Quad 2GB
> IT V1.1C". It is installed onto custom carrier board. I think, type of
> carrier board is not important, because issue happens even without real
> SPI slaves (with SPI loopback enabled).
> root@cr7:~# cat /proc/cpuinfo
> processor : 0
> model name : ARMv7 Processor rev 10 (v7l)
> BogoMIPS : 6.00
> Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpd32
> CPU implementer : 0x41
> CPU architecture: 7
> CPU variant : 0x2
> CPU part : 0xc09
> CPU revision : 10
>
> processor : 1
> model name : ARMv7 Processor rev 10 (v7l)
> BogoMIPS : 6.00
> Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpd32
> CPU implementer : 0x41
> CPU architecture: 7
> CPU variant : 0x2
> CPU part : 0xc09
> CPU revision : 10
>
> processor : 2
> model name : ARMv7 Processor rev 10 (v7l)
> BogoMIPS : 6.00
> Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpd32
> CPU implementer : 0x41
> CPU architecture: 7
> CPU variant : 0x2
> CPU part : 0xc09
> CPU revision : 10
>
> processor : 3
> model name : ARMv7 Processor rev 10 (v7l)
> BogoMIPS : 6.00
> Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpd32
> CPU implementer : 0x41
> CPU architecture: 7
> CPU variant : 0x2
> CPU part : 0xc09
> CPU revision : 10
>
> Hardware : Freescale i.MX6 Quad/DualLite (Device Tree)
> Revision :
> Serial : 05154315
>
> Linux host acts as SPI master, where different /dev/spidevX.Y files used
> for testing.
>
> The tools/spi/spidev_test.c from Linux kernel sources, compiled and used
> to test SPI driver from user-space.
> I use latest spidev_test.c (it does not change from 4.20.17 up to
> 5.1.0-rc2).
>
> My custom Device Tree contains description for spidev devices at 3 SPI
> bus, with 3 Chip Selects each:
> &ecspi1 { // SPI C for Option boards.
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_ecspi1>;
> cs-gpios = <&gpio5 25 GPIO_ACTIVE_HIGH>,
> <&gpio4 11 GPIO_ACTIVE_HIGH>,
> <&gpio3 25 GPIO_ACTIVE_HIGH>;
>
> spidev0_0: spidev@0_0 {
> status = "okay";
> compatible = "rohm,dh2228fv";
> reg = <0>;
> spi-max-frequency = <4000>;
> };
> spidev0_1: spidev@0_1 {
> status = "okay";
> compatible = "rohm,dh2228fv";
> reg = <1>;
> spi-max-frequency = <4000>;
> };
> spidev0_2: spidev@0_2 {
> status = "okay";
> compatible = "rohm,dh2228fv";
> reg = <2>;
> spi-max-frequency = <4000>;
> };
> };
> &ecspi2 { // SPI A for DSP.
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_ecspi2>;
> cs-gpios = <&gpio2 26 GPIO_ACTIVE_HIGH>,
> <&gpio2 27 GPIO_ACTIVE_HIGH>,
> <&gpio3 24 GPIO_ACTIVE_HIGH>;
>
> spidev1_0: spidev@1_0 {
> status = "okay";
> compatible = "rohm,dh2228fv";
> reg = <0>;
> spi-max-frequency = <4000>;
> };
> spidev1_1: spidev@1_1 {
> status = "okay";
> compatible = "rohm,dh2228fv";
> reg = <1>;
> spi-max-frequency = <4000>;
> };
> spidev1_2: spidev@1_2 {
> status = "okay";
> compatible = "rohm,dh2228fv";
> reg = <2>;
> spi-max-frequency = <4000>;
> };
> };
> &ecspi5 { // SPI B for DSP.
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_ecspi5>;
> cs-gpios = <&gpio1 17 GPIO_ACTIVE_HIGH>,
> <&gpio1 19 GPIO_ACTIVE_HIGH>,
> <&gpio1 21 GPIO_ACTIVE_HIGH>;
>
> spidev4_0: spidev@4_0 {
> status = "okay";
> compatible = "rohm,dh2228
RE: [RESEND] i2c: imx: defer probing on dma channel request
> From: laurentiu.tu...@nxp.com [mailto:laurentiu.tu...@nxp.com]
> Sent: Monday, March 25, 2019 11:30 PM
>
> From: Laurentiu Tudor
>
> If the dma controller is not yet probed, defer i2c probe.
> The error path in probe was slightly modified (no functional change) to avoid
> triggering this WARN_ON():
> "cg-pll0-div1 already disabled
> WARNING: CPU: 1 PID: 1 at drivers/clk/clk.c:828 clk_core_disable+0xa8/0xb0"
We may need more information to understand how this issue happens and it's also
better to put them in commit message to help review.
Regards
Dong Aisheng
>
> Signed-off-by: Laurentiu Tudor
> ---
> drivers/i2c/busses/i2c-imx.c | 21 +
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index
> 42fed40198a0..4e34b1572756 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -,7 +,8 @@ static int i2c_imx_probe(struct platform_device
> *pdev)
> pdev->name, i2c_imx);
> if (ret) {
> dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> - goto clk_disable;
> + clk_disable_unprepare(i2c_imx->clk);
> + return ret;
> }
>
> /* Init queue */
> @@ -1161,19 +1162,25 @@ static int i2c_imx_probe(struct platform_device
> *pdev)
> pm_runtime_mark_last_busy(&pdev->dev);
> pm_runtime_put_autosuspend(&pdev->dev);
>
> + /* Init DMA config if supported */
> + ret = i2c_imx_dma_request(i2c_imx, phy_addr);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + dev_info(&pdev->dev, "can't use DMA, using PIO
> instead.\n");
> + else
> + goto del_adapter;
> + }
> +
> dev_dbg(&i2c_imx->adapter.dev, "claimed irq %d\n", irq);
> dev_dbg(&i2c_imx->adapter.dev, "device resources: %pR\n", res);
> dev_dbg(&i2c_imx->adapter.dev, "adapter name: \"%s\"\n",
> i2c_imx->adapter.name);
>
> - /* Init DMA config if supported */
> - ret = i2c_imx_dma_request(i2c_imx, phy_addr);
> - if (ret < 0)
> - goto clk_notifier_unregister;
> -
> dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
> return 0; /* Return OK */
>
> +del_adapter:
> + i2c_del_adapter(&i2c_imx->adapter);
> clk_notifier_unregister:
> clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
> rpm_disable:
> @@ -1182,8 +1189,6 @@ static int i2c_imx_probe(struct platform_device
> *pdev)
> pm_runtime_set_suspended(&pdev->dev);
> pm_runtime_dont_use_autosuspend(&pdev->dev);
>
> -clk_disable:
> - clk_disable_unprepare(i2c_imx->clk);
> return ret;
> }
>
> --
> 2.17.1
RE: [PATCH v2 2/2] arm64: dts: imx8qxp: Add EDMA0/EDMA1 nodes
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > index 0cb939861a60..84c7c3eca1a1 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > @@ -182,6 +182,78 @@
> > > #clock-cells = <1>;
> > > };
> > >
> > > + edma0: dma-controller@591f {
> > > + compatible = "fsl,imx8qxp-edma";
> > > + reg = <0x5920 0x1>, /* asrc0 pair A
> input req */
> > > + <0x5921 0x1>, /* asrc0 pair B
> input req */
> > > + <0x5922 0x1>, /* asrc0 pair C
> input req */
> > > + <0x5923 0x1>, /* asrc0 pair A
> output req */
> > > + <0x5924 0x1>, /* asrc0 pair B
> output req */
> > > + <0x5925 0x1>, /* asrc0 pair C
> output req */
> > > + <0x5926 0x1>, /* esai0 rx */
> > > + <0x5927 0x1>, /* esai0 tx */
> > > + <0x5928 0x1>, /* spdif0 rx */
> > > + <0x5929 0x1>, /* spdif0 tx */
> > > + <0x592c 0x1>, /* sai0 rx */
> > > + <0x592d 0x1>, /* sai0 tx */
> > > + <0x592e 0x1>, /* sai1 rx */
> > > + <0x592f 0x1>, /* sai1 tx */
> > > + #dma-cells = <3>;
> >
> > In binding doc, it's 2.
> > - #dma-cells : Must be <2>.
> > The 1st cell specifies the DMAMUX(0 for DMAMUX0 and 1 for
> DMAMUX1).
> > Specific request source can only be multiplexed by specific
> channels
> > group called DMAMUX.
> > The 2nd cell specifies the request source(slot) ID.
> >
> > Need update binding doc?
> >
> > > + shared-interrupt;
> >
> > Undocumented property?
> >
> > Checkpatch did not complain?
>
> Thanks Aisheng for this comment. I think we might need to delay a little bit
> this patch because (as pointed by Yibin) on i.MX8 QXP/QM we use
> fsl-edma-v3 which *is not* yet upstream.
>
> So, lets get back to adding the nodes after Yibin sends edma-v3 patches.
>
Yes, Yibin could check the comments when resending with edma driver support.
Regards
Dong Aisheng
> thanks,
> Daniel.
RE: [RFC 0/7] cpuidle: Add poking mechanism to support non-IPI wakeup
> From: Marc Zyngier [mailto:marc.zyng...@arm.com] > Sent: Thursday, March 28, 2019 2:13 AM > On 27/03/2019 17:00, Leonard Crestez wrote: > > On Wed, 2019-03-27 at 17:06 +0100, Lucas Stach wrote: > >> Am Mittwoch, den 27.03.2019, 15:57 + schrieb Marc Zyngier: > >>> On 27/03/2019 15:44, Lucas Stach wrote: > Am Mittwoch, den 27.03.2019, 13:21 + schrieb Abel Vesa: > > This work is a workaround I'm looking into (more as a background > > task) in order to add support for cpuidle on i.MX8MQ based platforms. > > > > The main idea here is getting around the missing GIC wake_request > > signal (due to integration design issue) by waking up a each > > individual core through some dedicated SW power-up bits inside the > > power controller (GPC) right before every IPI is requested for that each > individual core. > > Just a general comment, without going into the details of this series: > this issue is not only affecting IPIs, but also MSIs terminated at > the GIC. Currently MSIs are terminated at the PCIe core, but > terminating them at the GIC is clearly preferable, as this allows > assigning CPU affinity to individual MSIs and lowers IRQ service > overhead. > > I'm not sure what the consequences are for upstream Linux support > yet, but we should keep in mind that having a workaround for IPIs > is only solving part of the issue. > >>> > >>> If this erratum is affecting more than just IPIs, then indeed I > >>> don't see how this patch series solves anything. > >>> > >>> But the erratum documentation seems to imply that only SGIs are > >>> affected, and goes as far as suggesting to use an external interrupt > >>> would solve it. How comes this is not the case? Or is it that > >>> anything directly routed to a redistributor is also affected? This > >>> would break LPIs (and thus MSIs) and PPIs (the CPU timer, among others). > >>> > >>> What is the *exact* status of this thing? I have the ugly feeling > >>> that the true workaround is just to disable cpuidle. > >> > >> As far as I understand the erratum, the basic issue is that the GIC > >> wake_request signals are not connected to the GPC (the CPU/peripheral > >> power sequencer). The SPIs are routed through the GPC and thus are > >> visible as wakeup sources, which is why the workaround of using an > >> external SPI as wakeup trigger for the IPI works. > > > > We had a kernel workaround for IPIs in our internal tree for a long > > time and I don't think we do anything special for PCI. Does PCI MSI > > really bypass the GPC on 8mq? > > If you have an ITS, certainly. If you don't, it depends. MSIs can hit the > distributor's MBI registers and generate non-wired SPIs, which I assume will > bypass the GPC altogether. > Richard & Jacky, Can you double check if this issue affect PCI MSI function? Regards Dong Aisheng > > Adding Richard/Jacky, they might know about this. > > > > This seems like something of a corner case to me, don't many imx > > boards ship without PCI; especially for low-power scenarios? If > > required it might be reasonable to add an additional workaround to > > disable all cpuidle if pci msis are used. > > Establishing a link between cpuidle and PCI in the kernel would be pretty > invasive, and that would come on top of what this series also mandates. > > At that level of apparent brokenness, it is far safer to get cpuidle out of > the > picture altogether, and I'd rather see these patches in a vendor tree (for > once). > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
RE: [RFC 0/7] cpuidle: Add poking mechanism to support non-IPI wakeup
[...] > > > Anything that isn't visible to the GPC and requires the GIC > > > wake_request signal to behave as specified is broken by this erratum. > > > > I really wonder how a timer interrupt (a PPI, hence not routed through > > the GPC) can wake up the CPU in this case. It really feels like > > something like "program CNTV_CVAL_EL0 to expire at some later point; > > WFI" could result in the CPU going to a deep sleep state, and not > > wake-up at all. > > I guess it's broken in the same way. The downstream DT claims > "local-timer-stop" for the CPU sleep state and "arm,no-tick-in-suspend" > for the armv8-timer, which I guess is not the timer actually stopping in > suspend, > but the CPU being unable to wake up due to the timer IRQ. > > > This would indicate that not only cpuidle is broken with this, but > > absolutely every interrupt that is not routed through the GPC. > > That's my understanding as well. Note that I have no NXP internal information > and can only infer from the published reference manual, errata notice and > downstream kernel. > We will double check it. Thanks for the information. Regards Dong Aisheng > Regards, > Lucas
RE: [RFC 0/7] cpuidle: Add poking mechanism to support non-IPI wakeup
[...] > > * All SPIs are connected to GPC in a 1:1 mapping > > * This series deals with SGIs > > * The timer PPIs are not required; covered by local-timer-stop > > * LPIs are currently unused (I understand imx-pci uses SPI by default > > from Lucas) > > > > Anything missing? > > > > My understanding is that this wake request feature via GIC is new in > > v3 and this is maybe why HW team missed it during integration. Older > > imx6/7 has GICv2 and has deep idle states which always rely on GPC to > > wakeup so the approach can work. > > Certainly the approach can work. The question is whether we want to support > this in a mainline kernel, spreading random hooks in the generic code and > adding a firmware interface on top of that. > > By all accounts, this HW is broken. You can indeed impose limitations (dumb > down PCI, mandate the use of a broadcast timer), or you can just flag cpuidle > as unsupported on this HW. My vote is on the latter. > Hi Marc, Rafael, Lorenzo Thanks for the suggestion. I fully understand the concern. Do you think we can patch the platform code to address the issue to avoid the big churn on kernel core code? If yes, we could try to investigate if there's a suitable place to do that. The main thing we need to do seems like to manually wakeup cpu core during the sending IPI path when exit idle. We could see if there's chance to do it on that path. Regards Dong Aisheng > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
RE: [PATCH V5 2/4] firmware: imx: enable imx scu general irq function
> From: Anson Huang
> Sent: Monday, March 18, 2019 11:10 AM
>
> The System Controller Firmware (SCFW) controls RTC, thermal and WDOG etc.,
> these resources' interrupt function are managed by SCU. When any IRQ
> pending, SCU will notify Linux via MU general interrupt channel #3, and Linux
> kernel needs to call SCU APIs to get IRQ status and notify each module to
> handle the interrupt.
>
> Since there is no data transmission for SCU IRQ notification, so doorbell mode
> is used for this MU channel, and SCU driver will use notifier mechanism to
> broadcast to every module which registers the SCU block notifier.
>
> Signed-off-by: Anson Huang
This patch mostly looks good me, except a few minor comments.
Otherwise, you can add my tag.
Reviewed-by: Dong Aisheng
> ---
> Changes since V4:
> - move scu irq support to a new file imx-scu-irq.c;
> - improve the function name with "imx_scu_irq_" as prefix for irq
> related
> functions;
> - move MU IRQ sources id out of sc ipc struct;
> - add IRQ group info message print when get irq status fail;
> - move MU ID parse into imx_scu_enable_general_irq_channel() function
> and ONLY do it
> when irq channel enable successfully.
> - change max IRQ group supported to 4, ONLY support for those kernel
> features.
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/imx-scu-irq.c | 133
> +
> drivers/firmware/imx/imx-scu.c | 6 ++
> include/linux/firmware/imx/sci.h | 4 ++
> 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/imx-scu-irq.c
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 1b2e15b..802c4ad 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o
> +obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o imx-scu-irq.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/imx-scu-irq.c
> b/drivers/firmware/imx/imx-scu-irq.c
> new file mode 100644
> index 000..0e20aa7
> --- /dev/null
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019 NXP
> + *
> + * Implementation of the SCU IRQ functions using MU.
> + *
> + */
> +
> +#include #include
> + #include
> +
> +#define IMX_SC_IRQ_FUNC_STATUS 2
> +#define IMX_SC_IRQ_NUM_GROUP 4
> +
> +static u32 mu_resource_id;
> +
> +struct imx_sc_msg_irq_get_status {
> + struct imx_sc_rpc_msg hdr;
> + union {
> + struct {
> + u16 resource;
> + u8 group;
> + u8 reserved;
> + } __packed req;
> + struct {
> + u32 status;
> + } resp;
> + } data;
> +};
> +
> +static struct imx_sc_ipc *imx_sc_irq_ipc_handle; static struct
> +work_struct imx_sc_irq_work; static
> +BLOCKING_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
> +
> +int imx_scu_irq_register_notifier(struct notifier_block *nb) {
> + return blocking_notifier_chain_register(
We probably can use atomic_notifier_call_chain as it's used for irq function.
> + &imx_scu_irq_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(imx_scu_irq_register_notifier);
> +
> +int imx_scu_irq_unregister_notifier(struct notifier_block *nb) {
> + return blocking_notifier_chain_unregister(
> + &imx_scu_irq_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(imx_scu_irq_unregister_notifier);
> +
> +static int imx_scu_irq_notifier_call_chain(unsigned long status, u8
> +*group) {
> + return blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
> + status, (void *)group);
> +}
> +
> +static void imx_scu_irq_work_handler(struct work_struct *work) {
> + struct imx_sc_msg_irq_get_status msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + u32 irq_status;
> + int ret;
> + u8 i;
> +
> + for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_IRQ;
> + hdr->func = IMX_SC_IRQ_FUNC_STATUS;
> + hdr->size = 2;
> +
> + msg.data.req.resource = mu_resource_id;
> + msg.data.req.group = i;
> +
> + ret = imx_scu_call_rpc(imx_sc_irq_ipc_handle, &msg, true);
> + if (ret) {
> + pr_err("get irq group %d status failed, ret %d\n",
> +i, ret);
> + return;
> + }
> +
> + irq_status = msg.data.resp.status;
> + if (!irq_status)
> + continue;
> +
> + imx_scu_irq_notifier_call_chain(irq_status, &i);
> + }
> +}
> +
> +static void imx_scu_irq_callback(struct mbox_client *c, void *msg) {
> + schedule_work(&imx_sc
RE: [PATCH V5 4/4] rtc: imx-sc: add rtc alarm support
> From: Anson Huang
> Sent: Monday, March 18, 2019 11:10 AM
>
> Add i.MX system controller RTC alarm support, the RTC alarm is implemented
> via SIP(silicon provider) runtime service call and ARM-Trusted-Firmware will
> communicate with system controller via MU(message unit) IPC to set RTC
> alarm. When RTC alarm fires, system controller will generate a common MU
> irq event and notify system controller RTC driver to handle the irq event.
>
> Signed-off-by: Anson Huang
> ---
> No function changes, just update imx_scu_irq_register_notifier() function
> name according to SCU IRQ function name change.
> ---
> drivers/rtc/rtc-imx-sc.c | 112
> +++
> 1 file changed, 112 insertions(+)
>
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c index
> 19642bf..9df4990 100644
> --- a/drivers/rtc/rtc-imx-sc.c
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -3,6 +3,7 @@
> * Copyright 2018 NXP.
> */
>
> +#include
> #include
> #include
> #include
> @@ -11,11 +12,17 @@
> #include
>
> #define IMX_SC_TIMER_FUNC_GET_RTC_SEC19709
> +#define IMX_SC_TIMER_FUNC_SET_RTC_ALARM 8
> #define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
>
> +#define IMX_SC_IRQ_FUNC_ENABLE 1
> +
> #define IMX_SIP_SRTC 0xC202
> #define IMX_SIP_SRTC_SET_TIME0x0
>
> +#define SC_IRQ_GROUP_RTC2
> +#define SC_IRQ_RTC 1
> +
> static struct imx_sc_ipc *rtc_ipc_handle; static struct rtc_device
> *imx_sc_rtc;
>
> @@ -24,6 +31,24 @@ struct imx_sc_msg_timer_get_rtc_time {
> u32 time;
> } __packed;
>
> +struct imx_sc_msg_timer_enable_irq {
> + struct imx_sc_rpc_msg hdr;
> + u32 mask;
> + u16 resource;
> + u8 group;
> + u8 enable;
> +} __packed;
> +
> +struct imx_sc_msg_timer_rtc_set_alarm {
> + struct imx_sc_rpc_msg hdr;
> + u16 year;
> + u8 mon;
> + u8 day;
> + u8 hour;
> + u8 min;
> + u8 sec;
> +} __packed;
> +
> static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm) {
> struct imx_sc_msg_timer_get_rtc_time msg; @@ -60,9 +85,92 @@ static
> int imx_sc_rtc_set_time(struct device *dev, struct rtc_time *tm)
> return res.a0;
> }
>
> +static int imx_sc_rtc_alarm_irq_enable(struct device *dev, unsigned int
> +enable) {
I think you shouldn't implement this generic function in rtc driver instead of
Imx-scu-irq driver.
> + struct imx_sc_msg_timer_enable_irq msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_IRQ;
> + hdr->func = IMX_SC_IRQ_FUNC_ENABLE;
> + hdr->size = 3;
> +
> + msg.resource = IMX_SC_R_MU_1A;
Here may be wrong as it is not align with what you did in Patch 2
that MU resource is dynamically detected.
> + msg.group = SC_IRQ_GROUP_RTC;
> + msg.mask = SC_IRQ_RTC;
> + msg.enable = enable;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + dev_err(dev, "enable rtc irq failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int imx_sc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm
> +*alrm) {
> + return 0;
> +}
Can't avoid define NULL function?
> +
> +static int imx_sc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm
> +*alrm) {
> + struct imx_sc_msg_timer_rtc_set_alarm msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> + struct rtc_time *alrm_tm = &alrm->time;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> + hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
> + hdr->size = 3;
> +
> + msg.year = alrm_tm->tm_year + 1900;
> + msg.mon = alrm_tm->tm_mon + 1;
> + msg.day = alrm_tm->tm_mday;
> + msg.hour = alrm_tm->tm_hour;
> + msg.min = alrm_tm->tm_min;
> + msg.sec = alrm_tm->tm_sec;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
Just curious we already have .alarm_irq_enable().
Why do we need call it again here?
Regards
Dong Aisheng
> + if (ret) {
> + dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static const struct rtc_class_ops imx_sc_rtc_ops = {
> .read_time = imx_sc_rtc_read_time,
> .set_time = imx_sc_rtc_set_time,
> + .read_alarm = imx_sc_rtc_read_alarm,
> + .set_alarm = imx_sc_rtc_set_alarm,
> + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable, };
> +
> +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> + unsigned long event, void *group)
> +{
> + /* ignore non-rtc irq */
> + if (!((event & SC_IRQ_R
RE: [PATCH v2] pinctrl: pinctrl-imx8mq: Add suspend/resume ops
> From: Abel Vesa
> Sent: Monday, April 8, 2019 6:43 PM
> Subject: [PATCH v2] pinctrl: pinctrl-imx8mq: Add suspend/resume ops
pinctrl: imx8mq:
> To support pinctl hog restore after LPSR resume back, add the generic
> suspend/resume in pinctrl-imx along with the generic pm opsto be used by
> platform specific drivers. Then make use of the newly added ops in i.MX8MQ
> platform specific driver.
>
> Signed-off-by: Robin Gong
> Signed-off-by: Abel Vesa
> ---
>
> Changes since v1:
> - Replaced the imx8mq specific ops with generic ones.
>
> drivers/pinctrl/freescale/pinctrl-imx.c| 25
> +
> drivers/pinctrl/freescale/pinctrl-imx.h| 4
> drivers/pinctrl/freescale/pinctrl-imx8mq.c | 1 +
> 3 files changed, 30 insertions(+)
>
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c
> b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 188001b..ee120ee 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -887,3 +887,28 @@ int imx_pinctrl_probe(struct platform_device *pdev,
>
> return ret;
> }
> +
> +int imx_pinctrl_suspend(struct device *dev) {
Static __maybe_unused
> + struct imx_pinctrl *ipctl = dev_get_drvdata(dev);
> +
> + if (!ipctl)
> + return -EINVAL;
I think we do not need this check as it's within the driver control.
> +
> + return pinctrl_force_sleep(ipctl->pctl); }
> +
> +int imx_pinctrl_resume(struct device *dev) {
Ditto
> + struct imx_pinctrl *ipctl = dev_get_drvdata(dev);
> +
> + if (!ipctl)
> + return -EINVAL;
Ditto
> +
> + return pinctrl_force_default(ipctl->pctl);
> +}
> +
> +const struct dev_pm_ops imx_pinctrl_pm_ops = {
> + SET_LATE_SYSTEM_SLEEP_PM_OPS(imx_pinctrl_suspend,
> + imx_pinctrl_resume)
> +};
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
> b/drivers/pinctrl/freescale/pinctrl-imx.h
> index 98a4889..ae78314 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> @@ -17,6 +17,7 @@
> struct platform_device;
>
> extern struct pinmux_ops imx_pmx_ops;
> +extern const struct dev_pm_ops imx_pinctrl_pm_ops;
>
> /**
> * struct imx_pin_mmio - MMIO pin configurations @@ -136,6 +137,9 @@
> struct imx_pinctrl { int imx_pinctrl_probe(struct platform_device *pdev,
> const struct imx_pinctrl_soc_info *info);
>
> +int imx_pinctrl_suspend(struct device *dev); int
> +imx_pinctrl_resume(struct device *dev);
Not need export this two functions
Regards
Dong Aisheng
> +
> #ifdef CONFIG_PINCTRL_IMX_SCU
> #define BM_PAD_CTL_GP_ENABLE BIT(30)
> #define BM_PAD_CTL_IFMUX_ENABLE BIT(31)
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mq.c
> b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
> index 8d39af5..50aa1c0 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx8mq.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
> @@ -339,6 +339,7 @@ static struct platform_driver imx8mq_pinctrl_driver =
> {
> .driver = {
> .name = "imx8mq-pinctrl",
> .of_match_table = of_match_ptr(imx8mq_pinctrl_of_match),
> + .pm = &imx_pinctrl_pm_ops,
> .suppress_bind_attrs = true,
> },
> .probe = imx8mq_pinctrl_probe,
> --
> 2.7.4
RE: [EXT] Re: [PATCH v2] dt-bindings: pinctrl: imx7d: Fix PAD_CTL_DSE_X*
> From: Linus Walleij [mailto:linus.wall...@linaro.org] > Sent: Monday, April 8, 2019 8:10 PM > On Wed, Apr 3, 2019 at 10:43 AM Shawn Guo wrote: > > > I assume this will go via your tree. Let me know if you think > > differently. > > OK I applied it, if you're not merging any device trees using these defines > this > cycle it's fine I suppose. > IMX device tree does not use it. So it's safe. Regards Dong Aisheng > Yours, > Linus Walleij
RE: [PATCH V5 4/4] rtc: imx-sc: add rtc alarm support
[...]
> so I will add another API in imx-scu-irq
> driver to provide function of enabling/disabling irq, each driver can just
> call the
> API to enable/disable its own IRQ, ONLY need to pass the corresponding
> arguments:
>
That's exactly what I mean.
> >
> > > + msg.group = SC_IRQ_GROUP_RTC;
> > > + msg.mask = SC_IRQ_RTC;
> > > + msg.enable = enable;
> > > +
> > > + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> > > + if (ret) {
> > > + dev_err(dev, "enable rtc irq failed, ret %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int imx_sc_rtc_read_alarm(struct device *dev, struct
> > > +rtc_wkalrm
> > > +*alrm) {
> > > + return 0;
> > > +}
> >
> > Can't avoid define NULL function?
>
> We have to implement it as NULL function, as SCFW does NOT provide such
> feature, But rtc alarm ONLY available when .read_alarm ops is implemented:
>
If the framework mandantorily requires it, then we'd probably better to add
doc before this function and explain why it's safe to be NULL.
> 147 static ssize_t
> 148 wakealarm_store(struct device *dev, struct device_attribute *attr,
> 149 const char *buf, size_t n)
> 150 {
>
> ...
>
> 187 retval = rtc_read_alarm(rtc, &alm);
> 188 if (retval < 0)
> 189 return retval;
>
>
> >
> > > +
> > > +static int imx_sc_rtc_set_alarm(struct device *dev, struct
> > > +rtc_wkalrm
> > > +*alrm) {
> > > + struct imx_sc_msg_timer_rtc_set_alarm msg;
> > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > + int ret;
> > > + struct rtc_time *alrm_tm = &alrm->time;
> > > +
> > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> > > + hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
> > > + hdr->size = 3;
> > > +
> > > + msg.year = alrm_tm->tm_year + 1900;
> > > + msg.mon = alrm_tm->tm_mon + 1;
> > > + msg.day = alrm_tm->tm_mday;
> > > + msg.hour = alrm_tm->tm_hour;
> > > + msg.min = alrm_tm->tm_min;
> > > + msg.sec = alrm_tm->tm_sec;
> > > +
> > > + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> > > + if (ret) {
> > > + dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
> >
> > Just curious we already have .alarm_irq_enable().
> > Why do we need call it again here?
>
> That is because the set_alarm function pass alarm time and alarm->enabled
> argument, it could be to enable alarm or to disable alarm, so we have to
> control the alarm function here, all other rtc drivers also do it this way.
> The .alarm_irq_enable() callback is for just enable or disable alarm.
>
Got it, thanks for the explanation.
Regards
Dong Aisheng
> Thanks,
> Anson
>
>
> >
> > Regards
> > Dong Aisheng
> >
> > > + if (ret) {
> > > + dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static const struct rtc_class_ops imx_sc_rtc_ops = {
> > > .read_time = imx_sc_rtc_read_time,
> > > .set_time = imx_sc_rtc_set_time,
> > > + .read_alarm = imx_sc_rtc_read_alarm,
> > > + .set_alarm = imx_sc_rtc_set_alarm,
> > > + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable, };
> > > +
> > > +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> > > + unsigned long event, void *group) {
> > > + /* ignore non-rtc irq */
> > > + if (!((event & SC_IRQ_RTC) && (*(u8 *)group ==
> > SC_IRQ_GROUP_RTC)))
> > > + return 0;
> > > +
> > > + rtc_update_irq(imx_sc_rtc, 1, RTC_IRQF | RTC_AF);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static struct notifier_block imx_sc_rtc_alarm_sc_notifier = {
> > > + .notifier_call = imx_sc_rtc_alarm_sc_notify,
> > > };
> > >
> > > static int imx_sc_rtc_probe(struct platform_device *pdev) @@ -73,6
> > > +181,8 @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
> > > if (ret)
> > > return ret;
> > >
> > > + device_init_wakeup(&pdev->dev, true);
> > > +
> > > imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> > > if (IS_ERR(imx_sc_rtc))
> > > return PTR_ERR(imx_sc_rtc);
> > > @@ -87,6 +197,8 @@ static int imx_sc_rtc_probe(struct
> > > platform_device
> > > *pdev)
> > > return ret;
> > > }
> > >
> > > + imx_scu_irq_register_notifier(&imx_sc_rtc_alarm_sc_notifier);
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.7.4
RE: [PATCH V6 2/4] firmware: imx: enable imx scu general irq function
> From: Anson Huang
> Sent: Tuesday, April 9, 2019 10:43 AM
> Subject: [PATCH V6 2/4] firmware: imx: enable imx scu general irq function
>
> The System Controller Firmware (SCFW) controls RTC, thermal and WDOG etc.,
> these resources' interrupt function are managed by SCU. When any IRQ
> pending, SCU will notify Linux via MU general interrupt channel #3, and Linux
> kernel needs to call SCU APIs to get IRQ status and notify each module to
> handle the interrupt.
>
> Since there is no data transmission for SCU IRQ notification, so doorbell mode
> is used for this MU channel, and SCU driver will use notifier mechanism to
> broadcast to every module which registers the SCU block notifier.
>
> Signed-off-by: Anson Huang
> ---
> Changes since V5:
> - use ATOMIC_NOTIFIER instead of BLOCKING_NOTIFIER for irq
> notification;
> - add memory free for failed case to avoid memory leak;
> - add new API imx_scu_irq_enable() for modules to enable/disable their
> own irqs.
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/imx-scu-irq.c | 166
> +
> drivers/firmware/imx/imx-scu.c | 6 ++
> include/linux/firmware/imx/sci.h | 5 ++
> 4 files changed, 178 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/imx-scu-irq.c
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 1b2e15b..802c4ad 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o
> +obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o imx-scu-irq.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/imx-scu-irq.c
> b/drivers/firmware/imx/imx-scu-irq.c
> new file mode 100644
> index 000..4000c63
> --- /dev/null
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019 NXP
> + *
> + * Implementation of the SCU IRQ functions using MU.
> + *
> + */
> +
> +#include #include
> + #include
> +
> +#define IMX_SC_IRQ_FUNC_ENABLE 1
> +#define IMX_SC_IRQ_FUNC_STATUS 2
> +#define IMX_SC_IRQ_NUM_GROUP 4
> +
> +static u32 mu_resource_id;
> +
> +struct imx_sc_msg_irq_get_status {
> + struct imx_sc_rpc_msg hdr;
> + union {
> + struct {
> + u16 resource;
> + u8 group;
> + u8 reserved;
> + } __packed req;
> + struct {
> + u32 status;
> + } resp;
> + } data;
> +};
> +
> +struct imx_sc_msg_irq_enable {
> + struct imx_sc_rpc_msg hdr;
> + u32 mask;
> + u16 resource;
> + u8 group;
> + u8 enable;
> +} __packed;
> +
> +static struct imx_sc_ipc *imx_sc_irq_ipc_handle; static struct
> +work_struct imx_sc_irq_work; static
> +ATOMIC_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
> +
> +int imx_scu_irq_register_notifier(struct notifier_block *nb) {
> + return atomic_notifier_chain_register(
> + &imx_scu_irq_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(imx_scu_irq_register_notifier);
> +
> +int imx_scu_irq_unregister_notifier(struct notifier_block *nb) {
> + return atomic_notifier_chain_unregister(
> + &imx_scu_irq_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(imx_scu_irq_unregister_notifier);
> +
> +static int imx_scu_irq_notifier_call_chain(unsigned long status, u8
> +*group) {
> + return atomic_notifier_call_chain(&imx_scu_irq_notifier_chain,
> + status, (void *)group);
> +}
> +
> +static void imx_scu_irq_work_handler(struct work_struct *work) {
> + struct imx_sc_msg_irq_get_status msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + u32 irq_status;
> + int ret;
> + u8 i;
> +
> + for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_IRQ;
> + hdr->func = IMX_SC_IRQ_FUNC_STATUS;
> + hdr->size = 2;
> +
> + msg.data.req.resource = mu_resource_id;
> + msg.data.req.group = i;
> +
> + ret = imx_scu_call_rpc(imx_sc_irq_ipc_handle, &msg, true);
> + if (ret) {
> + pr_err("get irq group %d status failed, ret %d\n",
> +i, ret);
> + return;
> + }
> +
> + irq_status = msg.data.resp.status;
> + if (!irq_status)
> + continue;
> +
> + imx_scu_irq_notifier_call_chain(irq_status, &i);
> + }
> +}
> +
> +void imx_scu_irq_enable(u8 group, u32 mask, u8 enable) {
Why should this be a void return?
Besides that, a nitpick better rename to imx_scu_irq_group_enable
to distinguish with the exist general irq enable.
Regards
Dong Aisheng
> + struct imx_sc_msg_irq_enable msg;
> + struct imx_sc_rpc_m
RE: [PATCH V6 4/4] rtc: imx-sc: add rtc alarm support
> From: Anson Huang
> Sent: Tuesday, April 9, 2019 10:44 AM
> Subject: [PATCH V6 4/4] rtc: imx-sc: add rtc alarm support
>
> Add i.MX system controller RTC alarm support, the RTC alarm is implemented
> via SIP(silicon provider) runtime service call and ARM-Trusted-Firmware will
> communicate with system controller via MU(message unit) IPC to set RTC
> alarm. When RTC alarm fires, system controller will generate a common MU
> irq event and notify system controller RTC driver to handle the irq event.
>
> Signed-off-by: Anson Huang
> ---
> Changes since V5:
> - move the irq alarm enable RPC to imx-scu-irq driver, and rtc driver
> just
> call the
> API to enable/disable alarm.
> ---
> drivers/rtc/rtc-imx-sc.c | 84
>
> 1 file changed, 84 insertions(+)
>
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c index
> 19642bf..3eb4db0 100644
> --- a/drivers/rtc/rtc-imx-sc.c
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -3,6 +3,7 @@
> * Copyright 2018 NXP.
> */
>
> +#include
> #include
> #include
> #include
> @@ -11,11 +12,15 @@
> #include
>
> #define IMX_SC_TIMER_FUNC_GET_RTC_SEC19709
> +#define IMX_SC_TIMER_FUNC_SET_RTC_ALARM 8
> #define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
>
> #define IMX_SIP_SRTC 0xC202
> #define IMX_SIP_SRTC_SET_TIME0x0
>
> +#define SC_IRQ_GROUP_RTC2
> +#define SC_IRQ_RTC 1
> +
> static struct imx_sc_ipc *rtc_ipc_handle; static struct rtc_device
> *imx_sc_rtc;
>
> @@ -24,6 +29,16 @@ struct imx_sc_msg_timer_get_rtc_time {
> u32 time;
> } __packed;
>
> +struct imx_sc_msg_timer_rtc_set_alarm {
> + struct imx_sc_rpc_msg hdr;
> + u16 year;
> + u8 mon;
> + u8 day;
> + u8 hour;
> + u8 min;
> + u8 sec;
> +} __packed;
> +
> static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm) {
> struct imx_sc_msg_timer_get_rtc_time msg; @@ -60,9 +75,74 @@ static
> int imx_sc_rtc_set_time(struct device *dev, struct rtc_time *tm)
> return res.a0;
> }
>
> +static int imx_sc_rtc_alarm_irq_enable(struct device *dev, unsigned int
> +enable) {
> + imx_scu_irq_enable(SC_IRQ_GROUP_RTC, SC_IRQ_RTC, enable);
> +
> + return 0;
> +}
> +
> +static int imx_sc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm
> +*alrm) {
I still think here needs a doc explain why needs this and why it's safe to do
that.
Otherwise:
Reviewed-by: Dong Aisheng
> + return 0;
> +}
> +
> +static int imx_sc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm
> +*alrm) {
> + struct imx_sc_msg_timer_rtc_set_alarm msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> + struct rtc_time *alrm_tm = &alrm->time;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> + hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
> + hdr->size = 3;
> +
> + msg.year = alrm_tm->tm_year + 1900;
> + msg.mon = alrm_tm->tm_mon + 1;
> + msg.day = alrm_tm->tm_mday;
> + msg.hour = alrm_tm->tm_hour;
> + msg.min = alrm_tm->tm_min;
> + msg.sec = alrm_tm->tm_sec;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
> + if (ret) {
> + dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static const struct rtc_class_ops imx_sc_rtc_ops = {
> .read_time = imx_sc_rtc_read_time,
> .set_time = imx_sc_rtc_set_time,
> + .read_alarm = imx_sc_rtc_read_alarm,
> + .set_alarm = imx_sc_rtc_set_alarm,
> + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable, };
> +
> +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> + unsigned long event, void *group)
Not necessary to have such a long function name.
Imx_sc_rtc_alarm_notify() should be ok
Regards
Dong Aisheng
> +{
> + /* ignore non-rtc irq */
> + if (!((event & SC_IRQ_RTC) && (*(u8 *)group == SC_IRQ_GROUP_RTC)))
> + return 0;
> +
> + rtc_update_irq(imx_sc_rtc, 1, RTC_IRQF | RTC_AF);
> +
> + return 0;
> +}
> +
> +static struct notifier_block imx_sc_rtc_alarm_sc_notifier = {
> + .notifier_call = imx_sc_rtc_alarm_sc_notify,
> };
>
> static int imx_sc_rtc_probe(struct platform_device *pdev) @@ -73,6 +153,8
> @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + device_init_wakeup(&pdev->dev, true);
> +
> imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> if (IS_ERR(imx_sc_rtc))
> return PTR_ERR(imx_sc_rtc);
> @@ -87,6 +169,8 @@ static int imx_sc_rtc_probe(struct platform_device
> *pdev)
> return ret;
> }
>
> +
RE: [PATCH v3] pinctrl: imx8mq: Add suspend/resume ops
> From: Abel Vesa > Sent: Tuesday, April 9, 2019 2:39 AM > > To support pinctl hog restore after LPSR resume back, add the generic > suspend/resume in pinctrl-imx along with the generic pm ops to be used by > platform specific drivers. Then make use of the newly added ops in i.MX8MQ > platform specific driver. > > Signed-off-by: Robin Gong > Signed-off-by: Abel Vesa Acked-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH V6 4/4] rtc: imx-sc: add rtc alarm support
[...]
> > > +static int imx_sc_rtc_alarm_irq_enable(struct device *dev, unsigned
> > > +int
> > > +enable) {
> > > + imx_scu_irq_enable(SC_IRQ_GROUP_RTC, SC_IRQ_RTC, enable);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int imx_sc_rtc_read_alarm(struct device *dev, struct
> > > +rtc_wkalrm
> > > +*alrm) {
> >
> > I still think here needs a doc explain why needs this and why it's
> > safe to do that.
>
> I will add a comment here, for the doc, it should be another topic of RTC
> framework, we can do it later.
I'm fine with it.
BTW, don't miss the next minor comments when you resend.
Regards
Dong Aisheng
> > > + .set_alarm = imx_sc_rtc_set_alarm,
> > > + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable, };
> > > +
> > > +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> > > + unsigned long event, void *group)
> >
> > Not necessary to have such a long function name.
> > Imx_sc_rtc_alarm_notify() should be ok
> >
RE: [RESEND] arm64: defconfig: Enable RTC_DRV_SNVS
> From: Abel Vesa > Sent: Tuesday, April 9, 2019 2:53 AM > > i.MX8MQ needs it for RTC support. > > Signed-off-by: Abel Vesa Reviewed-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH v2] clk: imx: Refactor entire sccg pll clk
> From: Stephen Boyd [mailto:sb...@kernel.org] > Sent: Tuesday, February 26, 2019 1:23 AM> > Quoting Abel Vesa (2019-02-23 02:58:14) > > On 19-02-22 09:46:23, Stephen Boyd wrote: > > > Quoting Abel Vesa (2019-02-22 09:07:32) > > > > Make the entire combination of plls to be one single clock. The > > > > parents used for bypasses are specified each as an index in the parents > list. > > > > The determine_rate does a lookup throughout all the possible > > > > combinations for all the divs and returns the best possible > > > > 'setup' which in turn is used by set_rate later to set up all the divs > > > > and > bypasses. > > > > > > > > Signed-off-by: Abel Vesa This significantly simply the clock tree output. Thanks for the work. > > > > Tested-by: Lucas Stach > > > > Acked-by: Lucas Stach > > > > > > I suspect these tested by and acked tags should have been dropped, > > > unless you discussed and tested off-list? > > > > > > > Oups, I forgot to drop them. > > Ok. Can Lucas re-test? Tested on clk-next by cherry-pick a few defconfig patches from imx tree. So: Tested-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH v2] clk: imx: Refactor entire sccg pll clk
[...] > > > > > > Tested-by: Lucas Stach > > > > > > Acked-by: Lucas Stach > > > > > > > > > > I suspect these tested by and acked tags should have been > > > > > dropped, unless you discussed and tested off-list? > > > > > > > > > > > > > Oups, I forgot to drop them. > > > > > > Ok. Can Lucas re-test? > > > > Tested on clk-next by cherry-pick a few defconfig patches from imx tree. > > So: > > Tested-by: Dong Aisheng > > > > Thanks for helping with the testing. > > I don't think there should be any defconfig changes required by this patch. > > Can you please list here the CONFIGs added? > > Just wanna make sure I'm not missing anything. > Clk-next does not have MX8 defconfig patches. So I manually cherry-picked them from imx-next. Below two needed: dee305e arm64: defconfig: Add i.MX8MQ boot necessary configs a1624ef arm64: defconfig: add imx8qxp support Regards Dong Aisheng > > Regards > > Dong Aisheng
RE: [PATCH RESEND V2 1/4] dt-bindings: fsl: scu: add watchdog binding
> From: Guenter Roeck [mailto:groe...@gmail.com] On Behalf Of Guenter
> Roeck>
> On 2/22/19 11:52 AM, Rob Herring wrote:
> > On Mon, Feb 18, 2019 at 06:53:48AM +, Anson Huang wrote:
> >> Add i.MX8QXP system controller watchdog binding.
> >>
> >> Signed-off-by: Anson Huang
> >> ---
> >> Changes since V1:
> >>- update dts node name to "watchdog";
> >> ---
> >> Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt | 10
> ++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> >> b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> >> index 4b79751..f388ec6 100644
> >> --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> >> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> >> @@ -136,6 +136,12 @@ Required properties:
> >> resource id for thermal driver to get temperature via
> >> SCU IPC.
> >>
> >> +Watchdog bindings based on SCU Message Protocol
> >> +
> >> +
> >> +Required properties:
> >> +- compatible: should be "fsl,imx8qxp-sc-wdt";
> >> +
> >> Example (imx8qxp):
> >> -
> >> lsio_mu1: mailbox@5d1c {
> >> @@ -188,6 +194,10 @@ firmware {
> >>tsens-num = <1>;
> >>#thermal-sensor-cells = <1>;
> >>};
> >> +
> >> + watchdog: watchdog {
> >> + compatible = "fsl,imx8qxp-sc-wdt";
> >
> > As-is, there's no reason for this to be in DT. The parent node's
> > driver can instantiate the wdog.
> >
>
> As the driver is currently written, you are correct, since it doesn't accept
> watchdog timeout configuration through devicetree.
>
> Question is if that is intended. Is it ?
>
Per my understanding, if the protocol is unlike to change, it can be
instantiated
by portent node's driver.
Hope Rob can help give a more general explanation.
Regards
Dong Aisheng
> Thanks,
> Guenter
RE: [PATCH V8 1/4] dt-bindings: fsl: scu: add thermal binding
[...]
> > On Thu, Feb 21, 2019 at 06:38:30AM +, Anson Huang wrote:
> > > NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system
> > > controller, the system controller is in charge of system power,
> > > clock and thermal sensors etc. management, Linux kernel has to
> > > communicate with system controller via MU (message unit) IPC to get
> > > temperature from thermal sensors, this patch adds binding doc for
> > > i.MX system controller thermal driver.
> > >
> > > Signed-off-by: Anson Huang
> > > Reviewed-by: Rob Herring
You need drop the former Reviewed-by once there's significant changes.
> > > +Thermal bindings based on SCU Message Protocol
> > > +
> > > +
> > > +Required properties:
> > > +- compatible:Should be :
> > > + "fsl,imx8qxp-sc-thermal"
> > > + followed by "fsl,imx-sc-thermal";
> > > +
> > > +- #thermal-sensor-cells: See
> > Documentation/devicetree/bindings/thermal/thermal.txt
> > > + for a description.
> > > +
> > > +- imx,sensor-resource-id:Property array to specify each thermal
> zone's
> > sensor resource ID.
> >
> > If this is an array...
> >
> > > +
> > > Example (imx8qxp):
> > > -
> > > lsio_mu1: mailbox@5d1c {
> > > @@ -168,6 +181,12 @@ firmware {
> > > rtc: rtc {
> > > compatible = "fsl,imx8qxp-sc-rtc";
> > > };
> > > +
> > > + tsens: thermal-sensor {
> > > + compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-
> > thermal";
> > > + #thermal-sensor-cells = <0>;
> > > + imx,sensor-resource-id = ;
> >
> > then this example should have an array, to be better express how this
> > is supposed to work. Can you please resend this with an array instead?
>
> On i.MX8QXP, there is ONLY 1 thermal zone available currently, but the thermal
> driver is supposed to support multi thermal zones as well for i.MX8QM, in V9
> patch set I just sent, I use the " imx,sensor-resource-id " element count to
> determine how many thermal sensors will be supported, so that we can also
> get rid of searching the thermal-zones' child node, I improved the description
> as below, is it OK?
>
Device tree is used to describe HW.
#thermal-sensor-cells is less to be an optional parameter to me.
If HW supports, then it should be 1 and update the example accordingly.
Regards
Dong Aisheng
> 41 +- imx,sensor-resource-id: A single integer for single thermal zone's
> resource ID or
> 42 + an array of integers to specify each
> thermal zone's sensor
> 43 + resource ID.
RE: [PATCH V4 1/2] dt-bindings: imx: remove unused resources from scu resource table
> From: Anson Huang > Sent: Saturday, February 23, 2019 11:20 AM > To: robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; > s.ha...@pengutronix.de; ker...@pengutronix.de; feste...@gmail.com; > ulf.hans...@linaro.org; Aisheng Dong ; > devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > linux-kernel@vger.kernel.org > Cc: dl-linux-imx > Subject: [PATCH V4 1/2] dt-bindings: imx: remove unused resources from scu > resource table > > Removes below resources which were defined during pre-silicon phase and the > real silicons do NOT have them, they have never been used, latest system > controller firmware also removed them: > > IMX_SC_R_DC_0_CAPTURE0 > IMX_SC_R_DC_0_CAPTURE1 > IMX_SC_R_DC_0_INTEGRAL0 > IMX_SC_R_DC_0_INTEGRAL1 > IMX_SC_R_DC_0_FRAC1 > IMX_SC_R_DC_1_CAPTURE0 > IMX_SC_R_DC_1_CAPTURE1 > IMX_SC_R_DC_1_INTEGRAL0 > IMX_SC_R_DC_1_INTEGRAL1 > IMX_SC_R_DC_1_FRAC1 > IMX_SC_R_GPU_3_PID0 > IMX_SC_R_M4_0_SIM > IMX_SC_R_M4_0_WDOG > IMX_SC_R_M4_1_SIM > IMX_SC_R_M4_1_WDOG > > Signed-off-by: Anson Huang There're gaps left after removing, but I can't see it's actually a problem. So: Reviewed-by: Dong Aisheng Regards Dong Aisheng > --- > Changes since V3: > - just remove those unused resource and no need to define UNUSED > resource. > -- > include/dt-bindings/firmware/imx/rsrc.h | 15 --- > 1 file changed, 15 deletions(-) > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h > b/include/dt-bindings/firmware/imx/rsrc.h > index 4481f2d..d69f934 100644 > --- a/include/dt-bindings/firmware/imx/rsrc.h > +++ b/include/dt-bindings/firmware/imx/rsrc.h > @@ -36,15 +36,10 @@ > #define IMX_SC_R_DC_0_BLIT1 20 > #define IMX_SC_R_DC_0_BLIT2 21 > #define IMX_SC_R_DC_0_BLIT_OUT 22 > -#define IMX_SC_R_DC_0_CAPTURE0 23 > -#define IMX_SC_R_DC_0_CAPTURE1 24 > #define IMX_SC_R_DC_0_WARP 25 > -#define IMX_SC_R_DC_0_INTEGRAL0 26 > -#define IMX_SC_R_DC_0_INTEGRAL1 27 > #define IMX_SC_R_DC_0_VIDEO0 28 > #define IMX_SC_R_DC_0_VIDEO1 29 > #define IMX_SC_R_DC_0_FRAC0 30 > -#define IMX_SC_R_DC_0_FRAC1 31 > #define IMX_SC_R_DC_032 > #define IMX_SC_R_GPU_2_PID0 33 > #define IMX_SC_R_DC_0_PLL_0 34 > @@ -53,17 +48,11 @@ > #define IMX_SC_R_DC_1_BLIT1 37 > #define IMX_SC_R_DC_1_BLIT2 38 > #define IMX_SC_R_DC_1_BLIT_OUT 39 > -#define IMX_SC_R_DC_1_CAPTURE0 40 > -#define IMX_SC_R_DC_1_CAPTURE1 41 > #define IMX_SC_R_DC_1_WARP 42 > -#define IMX_SC_R_DC_1_INTEGRAL0 43 > -#define IMX_SC_R_DC_1_INTEGRAL1 44 > #define IMX_SC_R_DC_1_VIDEO0 45 > #define IMX_SC_R_DC_1_VIDEO1 46 > #define IMX_SC_R_DC_1_FRAC0 47 > -#define IMX_SC_R_DC_1_FRAC1 48 > #define IMX_SC_R_DC_149 > -#define IMX_SC_R_GPU_3_PID0 50 > #define IMX_SC_R_DC_1_PLL_0 51 > #define IMX_SC_R_DC_1_PLL_1 52 > #define IMX_SC_R_SPI_0 53 > @@ -303,8 +292,6 @@ > #define IMX_SC_R_M4_0_UART 287 > #define IMX_SC_R_M4_0_I2C288 > #define IMX_SC_R_M4_0_INTMUX 289 > -#define IMX_SC_R_M4_0_SIM290 > -#define IMX_SC_R_M4_0_WDOG 291 > #define IMX_SC_R_M4_0_MU_0B 292 > #define IMX_SC_R_M4_0_MU_0A0 293 > #define IMX_SC_R_M4_0_MU_0A1 294 > @@ -323,8 +310,6 @@ > #define IMX_SC_R_M4_1_UART 307 > #define IMX_SC_R_M4_1_I2C308 > #define IMX_SC_R_M4_1_INTMUX 309 > -#define IMX_SC_R_M4_1_SIM310 > -#define IMX_SC_R_M4_1_WDOG 311 > #define IMX_SC_R_M4_1_MU_0B 312 > #define IMX_SC_R_M4_1_MU_0A0 313 > #define IMX_SC_R_M4_1_MU_0A1 314 > -- > 2.7.4
RE: [PATCH V4 2/2] dt-bindings: imx: add new resources to scu resource table
> From: Anson Huang > Sent: Saturday, February 23, 2019 11:20 AM > > Add new resources as below according to latest system controller firmware for > new features: > > IMX_SC_R_PERF > IMX_SC_R_OCRAM > IMX_SC_R_DMA_5_CH0 > IMX_SC_R_DMA_5_CH1 > IMX_SC_R_DMA_5_CH2 > IMX_SC_R_DMA_5_CH3 > IMX_SC_R_ATTESTATION > > Signed-off-by: Anson Huang > Reviewed-by: Rob Herring Please update the patch title to: dt-bindings: firmware: imx-scu: Otherwise: Reviewed-by: Dong Aisheng Regards Dong Aisheng > --- > No changes since V3. > --- > include/dt-bindings/firmware/imx/rsrc.h | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h > b/include/dt-bindings/firmware/imx/rsrc.h > index d69f934..4e61f64 100644 > --- a/include/dt-bindings/firmware/imx/rsrc.h > +++ b/include/dt-bindings/firmware/imx/rsrc.h > @@ -36,6 +36,7 @@ > #define IMX_SC_R_DC_0_BLIT1 20 > #define IMX_SC_R_DC_0_BLIT2 21 > #define IMX_SC_R_DC_0_BLIT_OUT 22 > +#define IMX_SC_R_PERF23 > #define IMX_SC_R_DC_0_WARP 25 > #define IMX_SC_R_DC_0_VIDEO0 28 > #define IMX_SC_R_DC_0_VIDEO1 29 > @@ -322,7 +323,7 @@ > #define IMX_SC_R_IRQSTR_SCU2 321 > #define IMX_SC_R_IRQSTR_DSP 322 > #define IMX_SC_R_ELCDIF_PLL 323 > -#define IMX_SC_R_UNUSED6 324 > +#define IMX_SC_R_OCRAM 324 > #define IMX_SC_R_AUDIO_PLL_0 325 > #define IMX_SC_R_PI_0326 > #define IMX_SC_R_PI_0_PWM_0 327 > @@ -539,6 +540,11 @@ > #define IMX_SC_R_VPU_MU_3538 > #define IMX_SC_R_VPU_ENC_1 539 > #define IMX_SC_R_VPU 540 > -#define IMX_SC_R_LAST541 > +#define IMX_SC_R_DMA_5_CH0 541 > +#define IMX_SC_R_DMA_5_CH1 542 > +#define IMX_SC_R_DMA_5_CH2 543 > +#define IMX_SC_R_DMA_5_CH3 544 > +#define IMX_SC_R_ATTESTATION 545 > +#define IMX_SC_R_LAST546 > > #endif /* __DT_BINDINGS_RSCRC_IMX_H */ > -- > 2.7.4
RE: [PATCH V4 1/2] dt-bindings: imx: remove unused resources from scu resource table
> > Subject: [PATCH V4 1/2] dt-bindings: imx: remove unused resources from > > scu resource table > > > > Removes below resources which were defined during pre-silicon phase > > and the real silicons do NOT have them, they have never been used, > > latest system controller firmware also removed them: > > > > IMX_SC_R_DC_0_CAPTURE0 > > IMX_SC_R_DC_0_CAPTURE1 > > IMX_SC_R_DC_0_INTEGRAL0 > > IMX_SC_R_DC_0_INTEGRAL1 > > IMX_SC_R_DC_0_FRAC1 > > IMX_SC_R_DC_1_CAPTURE0 > > IMX_SC_R_DC_1_CAPTURE1 > > IMX_SC_R_DC_1_INTEGRAL0 > > IMX_SC_R_DC_1_INTEGRAL1 > > IMX_SC_R_DC_1_FRAC1 > > IMX_SC_R_GPU_3_PID0 > > IMX_SC_R_M4_0_SIM > > IMX_SC_R_M4_0_WDOG > > IMX_SC_R_M4_1_SIM > > IMX_SC_R_M4_1_WDOG > > > > Signed-off-by: Anson Huang > > There're gaps left after removing, but I can't see it's actually a problem. > So: > Reviewed-by: Dong Aisheng > Sorry, missed to comment that please update the patch title: dt-bindings: firmware: imx-scu: Regards Dong Aisheng > Regards > Dong Aisheng > > > --- > > Changes since V3: > > - just remove those unused resource and no need to define UNUSED > > resource. > > -- > > include/dt-bindings/firmware/imx/rsrc.h | 15 --- > > 1 file changed, 15 deletions(-) > > > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h > > b/include/dt-bindings/firmware/imx/rsrc.h > > index 4481f2d..d69f934 100644 > > --- a/include/dt-bindings/firmware/imx/rsrc.h > > +++ b/include/dt-bindings/firmware/imx/rsrc.h > > @@ -36,15 +36,10 @@ > > #define IMX_SC_R_DC_0_BLIT120 > > #define IMX_SC_R_DC_0_BLIT221 > > #define IMX_SC_R_DC_0_BLIT_OUT 22 > > -#define IMX_SC_R_DC_0_CAPTURE0 23 > > -#define IMX_SC_R_DC_0_CAPTURE1 24 > > #define IMX_SC_R_DC_0_WARP 25 > > -#define IMX_SC_R_DC_0_INTEGRAL026 > > -#define IMX_SC_R_DC_0_INTEGRAL127 > > #define IMX_SC_R_DC_0_VIDEO0 28 > > #define IMX_SC_R_DC_0_VIDEO1 29 > > #define IMX_SC_R_DC_0_FRAC030 > > -#define IMX_SC_R_DC_0_FRAC131 > > #define IMX_SC_R_DC_0 32 > > #define IMX_SC_R_GPU_2_PID033 > > #define IMX_SC_R_DC_0_PLL_034 > > @@ -53,17 +48,11 @@ > > #define IMX_SC_R_DC_1_BLIT137 > > #define IMX_SC_R_DC_1_BLIT238 > > #define IMX_SC_R_DC_1_BLIT_OUT 39 > > -#define IMX_SC_R_DC_1_CAPTURE0 40 > > -#define IMX_SC_R_DC_1_CAPTURE1 41 > > #define IMX_SC_R_DC_1_WARP 42 > > -#define IMX_SC_R_DC_1_INTEGRAL043 > > -#define IMX_SC_R_DC_1_INTEGRAL144 > > #define IMX_SC_R_DC_1_VIDEO0 45 > > #define IMX_SC_R_DC_1_VIDEO1 46 > > #define IMX_SC_R_DC_1_FRAC047 > > -#define IMX_SC_R_DC_1_FRAC148 > > #define IMX_SC_R_DC_1 49 > > -#define IMX_SC_R_GPU_3_PID050 > > #define IMX_SC_R_DC_1_PLL_051 > > #define IMX_SC_R_DC_1_PLL_152 > > #define IMX_SC_R_SPI_0 53 > > @@ -303,8 +292,6 @@ > > #define IMX_SC_R_M4_0_UART 287 > > #define IMX_SC_R_M4_0_I2C 288 > > #define IMX_SC_R_M4_0_INTMUX 289 > > -#define IMX_SC_R_M4_0_SIM 290 > > -#define IMX_SC_R_M4_0_WDOG 291 > > #define IMX_SC_R_M4_0_MU_0B292 > > #define IMX_SC_R_M4_0_MU_0A0 293 > > #define IMX_SC_R_M4_0_MU_0A1 294 > > @@ -323,8 +310,6 @@ > > #define IMX_SC_R_M4_1_UART 307 > > #define IMX_SC_R_M4_1_I2C 308 > > #define IMX_SC_R_M4_1_INTMUX 309 > > -#define IMX_SC_R_M4_1_SIM 310 > > -#define IMX_SC_R_M4_1_WDOG 311 > > #define IMX_SC_R_M4_1_MU_0B312 > > #define IMX_SC_R_M4_1_MU_0A0 313 > > #define IMX_SC_R_M4_1_MU_0A1 314 > > -- > > 2.7.4
RE: [PATCH] Document: dt: binding: imx: Fix PAD_CTL_DSE_X*
> From: Christina Quast [mailto:cqu...@hanoverdisplays.com] > Sent: Saturday, February 23, 2019 1:01 AM > > In the iMX7d datasheet, the PAD_CTL_DSE_X* values are different from the > documentation. > It's a doc problem. Latest RM seems got updated. As here it's a reference definition in binding doc and device tree actually does not use it (IMX Pinctrl use raw data to set pad configuration). So it won't cause any compatibility issue to me. Please update the patch title to: dt-bindings: pinctrl: imx7d: x Otherwise: Ack-by: Dong Aisheng Regards Dong Aisheng > Signed-off-by: Christina Quast > --- > .../devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt > b/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt > index 277c3acb..8ac1d0851a0f 100644 > --- a/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt > +++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt > @@ -48,9 +48,9 @@ PAD_CTL_HYS (1 << 3) > PAD_CTL_SRE_SLOW(1 << 2) > PAD_CTL_SRE_FAST(0 << 2) > PAD_CTL_DSE_X1 (0 << 0) > -PAD_CTL_DSE_X2 (1 << 0) > -PAD_CTL_DSE_X3 (2 << 0) > -PAD_CTL_DSE_X4 (3 << 0) > +PAD_CTL_DSE_X4 (1 << 0) > +PAD_CTL_DSE_X2 (2 << 0) > +PAD_CTL_DSE_X6 (3 << 0) > > Examples: > While iomuxc-lpsr is intended to be used by dedicated peripherals to take > -- > 2.20.1 > > > > > > Please consider the environment before printing this email > > > > > > The information transmitted is intended only for the person or entity to which > it is addressed and may contain confidential and/or privileged material. Any > review, retransmission, dissemination or other use of, or taking of any > action in > reliance upon, this information by persons or entities other than the intended > recipient is prohibited. > If you received this in error, please contact the sender or postmaster > (postmas...@hanoverdisplays.com) and delete the material from any > computer. > Although we routinely screen for viruses, addressees should check this e-mail > and any attachment for viruses. We make no warranty as to absence of viruses > in this e-mail or any attachments. > Our Company's email policy is to permit incidental personal use. If this > email is > of a personal nature, it must not be relied upon as expressing the views or > opinions of the company.
RE: [PATCH V2 1/3] dt-bindings: mailbox: imx-mu: support i.MX8M
> From: Peng Fan > Sent: Monday, June 1, 2020 11:43 AM > > Add i.MX8MQ/M/N/P compatible string to support i.MX8M SoCs > > Signed-off-by: Peng Fan Reviewed-by: Dong Aisheng BTW, Anson, will you continue to help convert MU binding into json schemas? Regards Aisheng
RE: [PATCH V2 2/3] arm64: dts: imx8m: add mu node
> From: Peng Fan
> Sent: Monday, June 1, 2020 11:43 AM
>
> Add mu node to let A53 could communicate with M Core.
>
> Signed-off-by: Peng Fan
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 9 +
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 9 +
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 9 +
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 9 +
> 4 files changed, 36 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index aaf6e71101a1..fc001fb971e9 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -775,6 +775,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa {
> + compatible = "fsl,imx8mm-mu", "fsl,imx6sx-mu";
> + reg = <0x30aa 0x1>;
> + interrupts = ;
> + clocks = <&clk IMX8MM_CLK_MU_ROOT>;
> + clock-names = "mu";
You missed my comments about this unneeded line in the last round of review.
https://lore.kernel.org/patchwork/patch/1244752/
Regards
Aisheng
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b4 {
> compatible = "fsl,imx8mm-usdhc",
> "fsl,imx7d-usdhc";
> reg = <0x30b4 0x1>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> index 9a4b65a267d4..c8290d21ccc9 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -675,6 +675,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa {
> + compatible = "fsl,imx8mn-mu", "fsl,imx6sx-mu";
> + reg = <0x30aa 0x1>;
> + interrupts = ;
> + clocks = <&clk IMX8MN_CLK_MU_ROOT>;
> + clock-names = "mu";
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b4 {
> compatible = "fsl,imx8mn-usdhc",
> "fsl,imx7d-usdhc";
> reg = <0x30b4 0x1>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 45e2c0a4e889..b530804f763e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -621,6 +621,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa {
> + compatible = "fsl,imx8mp-mu", "fsl,imx6sx-mu";
> + reg = <0x30aa 0x1>;
> + interrupts = ;
> + clocks = <&clk IMX8MP_CLK_MU_ROOT>;
> + clock-names = "mu";
> + #mbox-cells = <2>;
> + };
> +
> i2c5: i2c@30ad {
> compatible = "fsl,imx8mp-i2c", "fsl,imx21-i2c";
> #address-cells = <1>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 978f8122c0d2..66ba8da704f6 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -959,6 +959,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa {
> + compatible = "fsl,imx8mq-mu", "fsl,imx6sx-mu";
> + reg = <0x30aa 0x1>;
> + interrupts = ;
> + clocks = <&clk IMX8MQ_CLK_MU_ROOT>;
> + clock-names = "mu";
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b4 {
> compatible = "fsl,imx8mq-usdhc",
>"fsl,imx7d-usdhc";
> --
> 2.16.4
RE: [PATCH V3 2/3] arm64: dts: imx8m: add mu node
> From: Peng Fan > Sent: Monday, June 1, 2020 4:20 PM > > Add mu node to let A53 could communicate with M Core. > > Signed-off-by: Peng Fan Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH] pinctrl: freescale: imx: Fix an error handling path in 'imx_pinctrl_probe()'
> From: Christophe JAILLET
> Sent: Sunday, May 31, 2020 4:50 AM
>
> 'pinctrl_unregister()' should not be called to undo
> 'devm_pinctrl_register_and_init()', it is already handled by the framework.
>
> This simplifies the error handling paths of the probe function.
> The 'imx_free_resources()' can be removed as well.
>
> Fixes: a51c158bf0f7 ("pinctrl: imx: use radix trees for groups and functions")
> Signed-off-by: Christophe JAILLET
Reviewed-by: Dong Aisheng
Regards
Aisheng
RE: [PATCH 2/4] firmware: imx: add resource management api
> From: Peng Fan > Sent: Monday, June 1, 2020 8:40 PM > > > > > From: Peng Fan > > > Sent: Friday, April 24, 2020 9:12 AM > > > > > > > > > From: Peng Fan > > > > > Sent: Thursday, April 23, 2020 6:57 PM > > > > > > > From: Peng Fan > > > > > > > Sent: Thursday, April 23, 2020 3:00 PM > > > > > > > > > > > > > > Add resource management API, when we have multiple partition > > > > > > > running together, resources not owned to current partition > > > > > > > should not be > > > > used. > > > > > > > > > > > > > > Reviewed-by: Leonard Crestez > > > > > > > Signed-off-by: Peng Fan > > > > > > > > > > > > Right now I'm still not quite understand if we really need this. > > > > > > As current resource is bound to power domains, if it's not > > > > > > owned by one specific SW execution environment, power on will also > fail. > > > > > > Can you clarify if any exceptions? > > > > > > > > > > There will be lots of failures when boot Linux domu if no check. > > > > > > > > > > > > > What kind of features did you mean? > > > > Could you clarify a bit more? I'd like to understand this issue better. > > > > > > When supporting hypervisor with dual Linux running, 1st Linux and > > > the 2nd Linux will have their own dedicated resources. > > > > > > If no resource check, that means 1st/2nd Linux will register all the > > > resources, then both will see fail logs when register resources not > > > owned to > > itself. > > > > > > Same to partitioned m4 case. > > > > > > Would it be better without the failure log? > > > > > > > Is it power up failure? > > If yes, it's expected because we actually use power up to check if > > resources are owned by this partition. It functions the same as > > calling resource check API. > > That's current design. > > > > Or it's other failures? Log? > Sorry for long late reply. > > Part of my XEN DomU log, there are lots of failure log. I think better not > have > such logs by add a resource own check. Those error logs are intended. Resource owner check actually behaves the same as power up. So not quite necessary to add a double check. If we're concerning about the error log, we can change it to dev_dbg(). BTW, as resource management will be needed by seco drivers later, So I will continue to review the patch. Regards Aisheng > > [2.034774] imx6q-pcie 5f00.pcie:IO 0x6ff8..0x6ff8 -> > 0x > [2.034801] imx6q-pcie 5f00.pcie: MEM 0x6000..0x6fef -> > 0x6000 > [2.035072] sdhc1: failed to power up resource 249 ret -13 > [2.035619] sdhc2: failed to power up resource 250 ret -13 > [2.036126] enet0: failed to power up resource 251 ret -13 > [2.036584] enet1: failed to power up resource 252 ret -13 > [2.037040] mlb0: failed to power up resource 253 ret -13 > [2.037495] nand: failed to power up resource 265 ret -13 > [2.037951] nand: failed to power up resource 265 ret -13 > [2.038416] pwm0: failed to power up resource 191 ret -13 > [2.038868] pwm1: failed to power up resource 192 ret -13 > [2.039320] pwm2: failed to power up resource 193 ret -13 > [2.039786] pwm3: failed to power up resource 194 ret -13 > [2.040239] pwm4: failed to power up resource 195 ret -13 > [2.040692] pwm5: failed to power up resource 196 ret -13 > [2.041142] pwm6: failed to power up resource 197 ret -13 > [2.041596] pwm7: failed to power up resource 198 ret -13 > [2.042073] amix: failed to power up resource 458 ret -13 > [2.042558] lpspi0: failed to power up resource 53 ret -13 > [2.043033] lpspi1: failed to power up resource 54 ret -13 > [2.043501] lpspi2: failed to power up resource 55 ret -13 > [2.043992] lpspi3: failed to power up resource 56 ret -13 > [2.044460] lpuart0: failed to power up resource 57 ret -13 > [2.044935] lpuart2: failed to power up resource 59 ret -13 > [2.045409] lpuart3: failed to power up resource 60 ret -13 > [2.055014] sim0: failed to power up resource 62 ret -13 > [2.055510] adc0: failed to power up resource 101 ret -13 > [2.056467] lpi2c0: failed to power up resource 96 ret -13 > [2.056946] lpi2c1: failed to power up resource 97 ret -13 > [2.057424] lpi2c2: failed to power up resource 98 ret -13 > [2.057898] lpi2c3: failed to power up resource 99 ret -13 > [2.058371] can0: failed to power up resource 105 ret -13 > [2.059289] can1: failed to power up resource 106 ret -13 > [2.059801] can2: failed to power up resource 107 ret -13 > [2.060281] nand: failed to power up resource 265 ret -13 > [2.062764] dpu-core 5618.dpu: driver probed > > Thanks, > Peng. > > > > > Regards > > Aisheng > > > > > Thanks, > > > Peng. > > > > > > > > > > > > > > Regards > > > > Aisheng > > > > > > > > > Thanks, > > > > > Peng. > > > > > > > > > > > > > > > > > Regards > > > > > > Aisheng > > > > > > > > > > > > > --- > > > > > > > drivers/firmware/imx/Makefile | 2 +- > > > > > > >
RE: [PATCH 2/4] firmware: imx: add resource management api
> From: Peng Fan
> Sent: Thursday, April 23, 2020 3:00 PM
>
> Add resource management API, when we have multiple partition running
> together, resources not owned to current partition should not be used.
>
> Reviewed-by: Leonard Crestez
> Signed-off-by: Peng Fan
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/rm.c | 40 +
> include/linux/firmware/imx/sci.h| 1 +
> include/linux/firmware/imx/svc/rm.h | 69
> +
> 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/rm.c create mode 100644
> include/linux/firmware/imx/svc/rm.h
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 08bc9ddfbdfb..17ea3613e142 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_IMX_DSP)+= imx-dsp.o
> -obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o imx-scu-irq.o
> +obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o imx-scu-irq.o rm.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c new file
> mode 100644 index ..7b0334de5486
> --- /dev/null
> +++ b/drivers/firmware/imx/rm.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 NXP
> + *
> + * File containing client-side RPC functions for the RM service. These
> + * function are ported to clients that communicate to the SC.
> + */
> +
> +#include
> +
> +struct imx_sc_msg_rm_rsrc_owned {
> + struct imx_sc_rpc_msg hdr;
> + u16 resource;
> +} __packed __aligned(4);
> +
> +/*
> + * This function check @resource is owned by current partition or not
> + *
> + * @param[in] ipc IPC handle
> + * @param[in] resourceresource the control is associated with
> + *
> + * @return Returns 0 for success and < 0 for errors.
> + */
> +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource)
> +{
> + struct imx_sc_msg_rm_rsrc_owned msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_RM;
> + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> + hdr->size = 2;
> +
> + msg.resource = resource;
> +
> + imx_scu_call_rpc(ipc, &msg, true);
No need check err return?
> +
> + return hdr->func;
> +}
> +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> diff --git a/include/linux/firmware/imx/sci.h
> b/include/linux/firmware/imx/sci.h
> index 17ba4e405129..b5c5a56f29be 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -15,6 +15,7 @@
>
> #include #include
>
> +#include
>
> int imx_scu_enable_general_irq_channel(struct device *dev); int
> imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> a/include/linux/firmware/imx/svc/rm.h b/include/linux/firmware/imx/svc/rm.h
> new file mode 100644
> index ..fc6ea62d9d0e
> --- /dev/null
> +++ b/include/linux/firmware/imx/svc/rm.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Copyright 2017-2019 NXP
Update copyright
> + *
> + * Header file containing the public API for the System Controller (SC)
> + * Power Management (PM) function. This includes functions for power
> +state
> + * control, clock control, reset control, and wake-up event control.
Fix the code comments
Otherwise, I'm fine with this patch.
Regards
Aisheng
> + *
> + * RM_SVC (SVC) Resource Management Service
> + *
> + * Module for the Resource Management (RM) service.
> + */
> +
> +#ifndef _SC_RM_API_H
> +#define _SC_RM_API_H
> +
> +#include
> +
> +/*
> + * This type is used to indicate RPC RM function calls.
> + */
> +enum imx_sc_rm_func {
> + IMX_SC_RM_FUNC_UNKNOWN = 0,
> + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> + IMX_SC_RM_FUNC_GET_DID = 26,
> + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> + IMX_SC_RM_FUNC_SET_PARENT = 6,
> + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
RE: [PATCH 1/2] ARM: dts: imx50: Add src node interrupt
> From: Anson Huang > Sent: Monday, May 18, 2020 8:54 PM > > Interrupt is a required property according to SRC binding, add it for SRC > node. > > Signed-off-by: Anson Huang Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH 2/2] ARM: dts: imx5: make src node name generic
> From: Anson Huang > Sent: Monday, May 18, 2020 8:54 PM > To: robh...@kernel.org; shawn...@kernel.org; s.ha...@pengutronix.de; > ker...@pengutronix.de; feste...@gmail.com; devicet...@vger.kernel.org; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org > Cc: dl-linux-imx > Subject: [PATCH 2/2] ARM: dts: imx5: make src node name generic > > Node name should be generic, use "reset-controller" instead of "src" for > i.MX5 SoCs src nodes. > > Signed-off-by: Anson Huang Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH V2] dt-bindings: clock: Convert i.MX7D clock to json-schema
> From: Anson Huang
> Sent: Monday, May 18, 2020 10:53 PM
>
> Convert the i.MX7D clock binding to DT schema format using json-schema.
>
> Signed-off-by: Anson Huang
> ---
> Changes since V1:
> - Update maintainer's e-mail address.
> ---
> .../devicetree/bindings/clock/imx7d-clock.txt | 13 -
> .../devicetree/bindings/clock/imx7d-clock.yaml | 64
> ++
> 2 files changed, 64 insertions(+), 13 deletions(-) delete mode 100644
> Documentation/devicetree/bindings/clock/imx7d-clock.txt
> create mode 100644
> Documentation/devicetree/bindings/clock/imx7d-clock.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> b/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> deleted file mode 100644
> index 9d3026d..000
> --- a/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -* Clock bindings for Freescale i.MX7 Dual
> -
> -Required properties:
> -- compatible: Should be "fsl,imx7d-ccm"
> -- reg: Address and length of the register set
> -- #clock-cells: Should be <1>
> -- clocks: list of clock specifiers, must contain an entry for each required
> - entry in clock-names
> -- clock-names: should include entries "ckil", "osc"
> -
> -The clock consumer should specify the desired clock by having the clock -ID
> in
> its "clocks" phandle cell. See include/dt-bindings/clock/imx7d-clock.h
> -for the full list of i.MX7 Dual clock IDs.
> diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> new file mode 100644
> index 000..8cd0573
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
> +
> +title: Clock bindings for Freescale i.MX7 Dual
> +
> +maintainers:
> + - Frank Li
> + - Anson Huang
> +
> +description: |
> + The clock consumer should specify the desired clock by having the
> +clock
> + ID in its "clocks" phandle cell. See
> +include/dt-bindings/clock/imx7d-clock.h
> + for the full list of i.MX7 Dual clock IDs.
> +
> +properties:
> + compatible:
> +const: fsl,imx7d-ccm
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +items:
> + - description: CCM interrupt request 1
> + - description: CCM interrupt request 2
Do we have a more specific description from RM?
Otherwise, I'm fine with this patch.
> +maxItems: 2
> +
> + '#clock-cells':
> +const: 1
> +
> + clocks:
> +items:
> + - description: 32k osc
> + - description: 24m osc
> +
> + clock-names:
> +items:
> + - const: ckil
> + - const: osc
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - '#clock-cells'
> +
> +examples:
> + - |
> +#include
> +
> +clock-controller@3038 {
> +compatible = "fsl,imx7d-ccm";
> +reg = <0x3038 0x1>;
> +interrupts = ,
> + ;
> +#clock-cells = <1>;
> +clocks = <&ckil>, <&osc>;
> +clock-names = "ckil", "osc";
> +};
> --
> 2.7.4
RE: [PATCH 1/3] dt-bindings: timer: Convert i.MX GPT to json-schema
> From: Anson Huang
> Sent: Monday, May 18, 2020 10:48 PM
>
> Convert the i.MX GPT binding to DT schema format using json-schema.
>
> Signed-off-by: Anson Huang
> ---
> .../devicetree/bindings/timer/fsl,imxgpt.txt | 45 -
> .../devicetree/bindings/timer/fsl,imxgpt.yaml | 109
> +
> 2 files changed, 109 insertions(+), 45 deletions(-) delete mode 100644
> Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> create mode 100644
> Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
>
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> b/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> deleted file mode 100644
> index 5d8fd5b..000
> --- a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -Freescale i.MX General Purpose Timer (GPT)
> -
> -Required properties:
> -
> -- compatible : should be one of following:
> - for i.MX1:
> - - "fsl,imx1-gpt";
> - for i.MX21:
> - - "fsl,imx21-gpt";
> - for i.MX27:
> - - "fsl,imx27-gpt", "fsl,imx21-gpt";
> - for i.MX31:
> - - "fsl,imx31-gpt";
> - for i.MX25:
> - - "fsl,imx25-gpt", "fsl,imx31-gpt";
> - for i.MX50:
> - - "fsl,imx50-gpt", "fsl,imx31-gpt";
> - for i.MX51:
> - - "fsl,imx51-gpt", "fsl,imx31-gpt";
> - for i.MX53:
> - - "fsl,imx53-gpt", "fsl,imx31-gpt";
> - for i.MX6Q:
> - - "fsl,imx6q-gpt", "fsl,imx31-gpt";
> - for i.MX6DL:
> - - "fsl,imx6dl-gpt";
> - for i.MX6SL:
> - - "fsl,imx6sl-gpt", "fsl,imx6dl-gpt";
> - for i.MX6SX:
> - - "fsl,imx6sx-gpt", "fsl,imx6dl-gpt";
> -- reg : specifies base physical address and size of the registers.
> -- interrupts : should be the gpt interrupt.
> -- clocks : the clocks provided by the SoC to drive the timer, must contain
> - an entry for each entry in clock-names.
> -- clock-names : must include "ipg" entry first, then "per" entry.
> -
> -Example:
> -
> -gpt1: timer@10003000 {
> - compatible = "fsl,imx27-gpt", "fsl,imx21-gpt";
> - reg = <0x10003000 0x1000>;
> - interrupts = <26>;
> - clocks = <&clks IMX27_CLK_GPT1_IPG_GATE>,
> - <&clks IMX27_CLK_PER1_GATE>;
> - clock-names = "ipg", "per";
> -};
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> new file mode 100644
> index 000..5c7186b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
> +
> +title: Freescale i.MX General Purpose Timer (GPT)
> +
> +maintainers:
> + - Sascha Hauer
> +
> +properties:
> + compatible:
> +oneOf:
> + - description: on i.MX1 the following compatible must be specified
Such a description seems not necessary in order to make the doc more clean
> +items:
> + - const: "fsl,imx1-gpt"
> +
> + - description: on i.MX21 the following compatible must be specified
> +items:
> + - const: "fsl,imx21-gpt"
> +
> + - description: on i.MX27 the following compatibles must be specified
> +items:
> + - const: "fsl,imx27-gpt"
> + - const: "fsl,imx21-gpt"
> +
> + - description: on i.MX31 the following compatible must be specified
> +items:
> + - const: "fsl,imx31-gpt"
> +
> + - description: on i.MX25 the following compatibles must be specified
> +items:
> + - const: "fsl,imx25-gpt"
> + - const: "fsl,imx31-gpt"
> +
> + - description: on i.MX50 the following compatibles must be specified
> +items:
> + - const: "fsl,imx50-gpt"
> + - const: "fsl,imx31-gpt"
> +
> + - description: on i.MX51 the following compatibles must be specified
> +items:
> + - const: "fsl,imx51-gpt"
> + - const: "fsl,imx31-gpt"
> +
> + - description: on i.MX53 the following compatibles must be specified
> +items:
> + - const: "fsl,imx53-gpt"
> + - const: "fsl,imx31-gpt"
> +
> + - description: on i.MX6Q the following compatibles must be specified
> +items:
> + - const: "fsl,imx6q-gpt"
> + - const: "fsl,imx31-gpt"
> +
> + - description: on i.MX6DL the following compatible must be specified
> +items:
> + - const: "fsl,imx6dl-gpt"
> +
> + - description: on i.MX6SL the following compatibles must be specified
> +items:
> + - const: "fsl,imx6sl-gpt"
> + - const: "fsl,imx6dl-gpt"
> +
> + - description: on i.MX6SX the following compatibles must be specified
> +items:
> + - const: "fsl,imx6sx-gpt"
> + - const: "fsl,imx6dl-gpt"
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +maxItems: 1
> +
> + clocks:
> +items:
> + - description: SoC GPT ipg clock
> + - description: SoC GPT per clock
> +maxItems: 2
maxItems is not needed
> +
> + clock-names:
> +it
RE: [PATCH 2/3] dt-bindings: timer: Convert i.MX TPM to json-schema
> From: Anson Huang
> Sent: Monday, May 18, 2020 10:48 PM
>
> Convert the i.MX TPM binding to DT schema format using json-schema.
>
> Signed-off-by: Anson Huang
> ---
> .../devicetree/bindings/timer/nxp,tpm-timer.txt| 28 --
> .../devicetree/bindings/timer/nxp,tpm-timer.yaml | 63
> ++
> 2 files changed, 63 insertions(+), 28 deletions(-) delete mode 100644
> Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> create mode 100644
> Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
>
> diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> deleted file mode 100644
> index f82087b..000
> --- a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -NXP Low Power Timer/Pulse Width Modulation Module (TPM)
> -
> -The Timer/PWM Module (TPM) supports input capture, output compare, -and
> the generation of PWM signals to control electric motor and power
> -management applications. The counter, compare and capture registers -are
> clocked by an asynchronous clock that can remain enabled in low -power modes.
> TPM can support global counter bus where one TPM drives -the counter bus for
> the others, provided bit width is the same.
> -
> -Required properties:
> -
> -- compatible : should be "fsl,imx7ulp-tpm"
> -- reg : Specifies base physical address and size of the
> register sets
> - for the clock event device and clock source device.
> -- interrupts : Should be the clock event device interrupt.
> -- clocks : The clocks provided by the SoC to drive the timer, must contain
> - an entry for each entry in clock-names.
> -- clock-names : Must include the following entries: "ipg" and "per".
> -
> -Example:
> -tpm5: tpm@4026 {
> - compatible = "fsl,imx7ulp-tpm";
> - reg = <0x4026 0x1000>;
> - interrupts = ;
> - clocks = <&clks IMX7ULP_CLK_NIC1_BUS_DIV>,
> - <&clks IMX7ULP_CLK_LPTPM5>;
> - clock-names = "ipg", "per";
> -};
> diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
> b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
> new file mode 100644
> index 000..0d34610
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
[...]
> +
> +title: NXP Low Power Timer/Pulse Width Modulation Module (TPM)
> +
> +maintainers:
> + - Dong Aisheng
> +
> +description: |
> + The Timer/PWM Module (TPM) supports input capture, output compare,
> + and the generation of PWM signals to control electric motor and power
> + management applications. The counter, compare and capture registers
> + are clocked by an asynchronous clock that can remain enabled in low
> + power modes. TPM can support global counter bus where one TPM drives
> + the counter bus for the others, provided bit width is the same.
> +
> +properties:
> + compatible:
> +const: fsl,imx7ulp-tpm
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +maxItems: 1
> +
> + clocks:
> +items:
> + - description: SoC TPM ipg clock
> + - description: SoC TPM per clock
> +maxItems: 2
Unneeded line
> +
> + clock-names:
> +items:
> + - const: ipg
> + - const: per
> +maxItems: 2
Ditto
Otherwise:
Reviewed-by: Dong Aisheng
Regards
Aisheng
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +
> +timer@4026 {
> +compatible = "fsl,imx7ulp-tpm";
> +reg = <0x4026 0x1000>;
> +interrupts = ;
> +clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
> + <&pcc2 IMX7ULP_CLK_LPTPM5>;
> +clock-names = "ipg", "per";
> +};
> --
> 2.7.4
RE: [PATCH 3/3] dt-bindings: timer: Convert i.MX SYSCTR to json-schema
> From: Anson Huang > Sent: Monday, May 18, 2020 10:48 PM > > Convert the i.MX SYSCTR binding to DT schema format using json-schema. > > Signed-off-by: Anson Huang Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH] ARM: dts: imx: make src node name generic
> From: Anson Huang
> Sent: Monday, May 18, 2020 8:40 PM
>
> Node name should be generic, use "reset-controller" instead of "src" for
> i.MX6/i.MX7 SoCs src nodes.
>
> Signed-off-by: Anson Huang
Reviewed-by: Dong Aisheng
Regards
Aisheng
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
> arch/arm/boot/dts/imx6sl.dtsi | 2 +-
> arch/arm/boot/dts/imx6sx.dtsi | 2 +-
> arch/arm/boot/dts/imx6ul.dtsi | 2 +-
> arch/arm/boot/dts/imx7s.dtsi | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 1763c2b..39d4afd 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -858,7 +858,7 @@
> interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> - src: src@20d8000 {
> + src: reset-controller@20d8000 {
> compatible = "fsl,imx6q-src", "fsl,imx51-src";
> reg = <0x020d8000 0x4000>;
> interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>, diff
> --git
> a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi index
> fcb84fe..911d8cf 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -678,7 +678,7 @@
> interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> - src: src@20d8000 {
> + src: reset-controller@20d8000 {
> compatible = "fsl,imx6sl-src", "fsl,imx51-src";
> reg = <0x020d8000 0x4000>;
> interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>, diff
> --git
> a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index
> d6f8317..e031337 100644
> --- a/arch/arm/boot/dts/imx6sx.dtsi
> +++ b/arch/arm/boot/dts/imx6sx.dtsi
> @@ -754,7 +754,7 @@
> interrupts = ;
> };
>
> - src: src@20d8000 {
> + src: reset-controller@20d8000 {
> compatible = "fsl,imx6sx-src", "fsl,imx51-src";
> reg = <0x020d8000 0x4000>;
> interrupts = ,
> diff --git
> a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index
> 2ccf67c..35e7301 100644
> --- a/arch/arm/boot/dts/imx6ul.dtsi
> +++ b/arch/arm/boot/dts/imx6ul.dtsi
> @@ -676,7 +676,7 @@
> interrupts = ;
> };
>
> - src: src@20d8000 {
> + src: reset-controller@20d8000 {
> compatible = "fsl,imx6ul-src", "fsl,imx51-src";
> reg = <0x020d8000 0x4000>;
> interrupts = ,
> diff --git
> a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi index
> 76e3ffb..8bac491 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -624,7 +624,7 @@
> clock-names = "ckil", "osc";
> };
>
> - src: src@3039 {
> + src: reset-controller@3039 {
> compatible = "fsl,imx7d-src", "syscon";
> reg = <0x3039 0x1>;
> interrupts = ;
> --
> 2.7.4
RE: [PATCH V2] dt-bindings: reset: Convert i.MX reset to json-schema
> From: Anson Huang
> Sent: Monday, May 18, 2020 6:04 PM
>
> Convert the i.MX reset binding to DT schema format using json-schema.
>
> Signed-off-by: Anson Huang
> ---
> Changes since V1:
> - add the compatible strings per SoC;
> - use "reset-controller" as node name instead of src;
> - add interrupt items description.
> ---
> .../devicetree/bindings/reset/fsl,imx-src.txt | 49 ---
> .../devicetree/bindings/reset/fsl,imx-src.yaml | 98
> ++
> 2 files changed, 98 insertions(+), 49 deletions(-) delete mode 100644
> Documentation/devicetree/bindings/reset/fsl,imx-src.txt
> create mode 100644
> Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
>
> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
> b/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
> deleted file mode 100644
> index 6ed79e6..000
> --- a/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -Freescale i.MX System Reset Controller
> -==
> -
> -Please also refer to reset.txt in this directory for common reset -controller
> binding usage.
> -
> -Required properties:
> -- compatible: Should be "fsl,-src"
> -- reg: should be register base and length as documented in the
> - datasheet
> -- interrupts: Should contain SRC interrupt and CPU WDOG interrupt,
> - in this order.
> -- #reset-cells: 1, see below
> -
> -example:
> -
> -src: src@20d8000 {
> -compatible = "fsl,imx6q-src";
> -reg = <0x020d8000 0x4000>;
> -interrupts = <0 91 0x04 0 96 0x04>;
> -#reset-cells = <1>;
> -};
> -
> -Specifying reset lines connected to IP modules
> -==
> -
> -The system reset controller can be used to reset the GPU, VPU, -IPU, and
> OpenVG IP modules on i.MX5 and i.MX6 ICs. Those device -nodes should
> specify the reset line on the SRC in their resets -property, containing a
> phandle
> to the SRC device node and a -RESET_INDEX specifying which module to reset,
> as described in -reset.txt
> -
> -example:
> -
> -ipu1: ipu@240 {
> -resets = <&src 2>;
> -};
> -ipu2: ipu@280 {
> -resets = <&src 4>;
> -};
> -
> -The following RESET_INDEX values are valid for i.MX5:
> -GPU_RESET 0
> -VPU_RESET 1
> -IPU1_RESET2
> -OPEN_VG_RESET 3
> -The following additional RESET_INDEX value is valid for i.MX6:
> -IPU2_RESET4
> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
> b/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
> new file mode 100644
> index 000..7cd6095
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
[...]
> +title: Freescale i.MX System Reset Controller
> +
> +maintainers:
> + - Philipp Zabel
> +
> +description: |
> + The system reset controller can be used to reset the GPU, VPU,
> + IPU, and OpenVG IP modules on i.MX5 and i.MX6 ICs. Those device
> + nodes should specify the reset line on the SRC in their resets
> + property, containing a phandle to the SRC device node and a
> + RESET_INDEX specifying which module to reset, as described in
> + reset.txt
> +
> + The following RESET_INDEX values are valid for i.MX5:
> +GPU_RESET 0
> +VPU_RESET 1
> +IPU1_RESET2
> +OPEN_VG_RESET 3
> + The following additional RESET_INDEX value is valid for i.MX6:
> +IPU2_RESET4
> +
> +properties:
> + compatible:
> +oneOf:
> + - description: on i.MX51 the following compatible must be specified
Unnecessary description
Otherwise:
Reviewed-by: Dong Aisheng
Regards
Aisheng
> +items:
> + - const: "fsl,imx51-src"
> +
> + - description: on i.MX50 the following compatibles must be specified
> +items:
> + - const: "fsl,imx50-src"
> + - const: "fsl,imx51-src"
> +
> + - description: on i.MX53 the following compatibles must be specified
> +items:
> + - const: "fsl,imx53-src"
> + - const: "fsl,imx51-src"
> +
> + - description: on i.MX6Q the following compatibles must be specified
> +items:
> + - const: "fsl,imx6q-src"
> + - const: "fsl,imx51-src"
> +
> + - description: on i.MX6SX the following compatibles must be specified
> +items:
> + - const: "fsl,imx6sx-src"
> + - const: "fsl,imx51-src"
> +
> + - description: on i.MX6SL the following compatibles must be specified
> +items:
> + - const: "fsl,imx6sl-src"
> + - const: "fsl,imx51-src"
> +
> + - description: on i.MX6UL the following compatibles must be specified
> +items:
> + - const: "fsl,imx6ul-src"
> + - const: "fsl,imx51-src"
> +
> + - description: on i.MX6SLL the following compatibles must be
RE: [PATCH V2 1/3] dt-bindings: timer: Convert i.MX GPT to json-schema
> From: Anson Huang > Sent: Tuesday, May 19, 2020 11:56 AM > > Convert the i.MX GPT binding to DT schema format using json-schema. > > Signed-off-by: Anson Huang Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH 0/4] arm64: dts: imx8m: dtb aliases update
> From: Peng Fan > Sent: Wednesday, May 20, 2020 10:03 AM > > Minor patchset to update device tree aliases > > Peng Fan (4): > arm64: dts: imx8mq: Add mmc aliases > arm64: dts: imx8mq: Add ethernet alias > arm64: dts: imx8mm: sort the aliases > arm64: dts: imx8mp: add i2c aliases For this patchset, Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH 1/3] arm64: dts: imx8m: add mu node
> From: Peng Fan
> Sent: Wednesday, May 20, 2020 10:05 AM
>
> Add mu node to let A53 could communicate with M Core.
>
> Signed-off-by: Peng Fan
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 9 +
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 9 +
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 9 +
> 3 files changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index f3bbefe3e59f..9722f76d8c3f 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -771,6 +771,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa {
> + compatible = "fsl,imx6sx-mu";
Usually we also add current SoC compatible string.
compatible = "fsl,imx8mm-mu", "fsl,imx6sx-mu"
> + reg = <0x30aa 0x1>;
> + interrupts = ;
> + clocks = <&clk IMX8MM_CLK_MU_ROOT>;
> + clock-names = "mu";
Undocumented property, drop it
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b4 {
> compatible = "fsl,imx8mm-usdhc",
> "fsl,imx7d-usdhc";
> reg = <0x30b4 0x1>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> index fb63a98fdff5..5f30f1d50460 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -671,6 +671,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa {
> + compatible = "fsl,imx6sx-mu";
> + reg = <0x30aa 0x1>;
> + interrupts = ;
> + clocks = <&clk IMX8MN_CLK_MU_ROOT>;
> + clock-names = "mu";
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b4 {
> compatible = "fsl,imx8mn-usdhc",
> "fsl,imx7d-usdhc";
> reg = <0x30b4 0x1>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 1d15680a4962..e969fcbbd15f 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -956,6 +956,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa {
> + compatible = "fsl,imx6sx-mu";
> + reg = <0x30aa 0x1>;
> + interrupts = ;
> + clocks = <&clk IMX8MQ_CLK_MU_ROOT>;
> + clock-names = "mu";
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b4 {
> compatible = "fsl,imx8mq-usdhc",
>"fsl,imx7d-usdhc";
> --
> 2.16.4
RE: [PATCH 2/3] clk: imx8mp: add mu root clk
> From: Peng Fan > Sent: Wednesday, May 20, 2020 10:05 AM > > Add mu root clk for mu mailbox usage. > > Signed-off-by: Peng Fan Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH 3/3] arm64: dts: imx8mp: add mu node
> From: Fabio Estevam > Sent: Wednesday, May 20, 2020 11:07 AM > > Hi Peng, > > On Wed, May 20, 2020 at 12:01 AM Peng Fan wrote: > > > Nothing specific in i.MX8MP for the mu part, so do we really need add > > "fsl,imx8mp-mu"? > > It is good practice to add a more specific option. > > Let's say in future a bug is found that affects imx8mp MU, then you could fix > the > MU driver and keep the dtb compatibility. +1
RE: [PATCH V2] dt-bindings: thermal: Convert i.MX to json-schema
> From: Anson Huang
> Sent: Wednesday, May 20, 2020 2:03 PM
>
> Convert the i.MX thermal binding to DT schema format using json-schema
>
> Signed-off-by: Anson Huang
> ---
> Changes since V1:
> - move tempmon node into its parent node anatop in example;
> - improve "fsl,tempmon" description.
> ---
> .../devicetree/bindings/thermal/imx-thermal.txt| 61 -
> .../devicetree/bindings/thermal/imx-thermal.yaml | 100
[...]
> +title: NXP i.MX Thermal Binding
> +
> +maintainers:
> + - Shawn Guo
> + - Anson Huang
> +
> +properties:
> + compatible:
> +oneOf:
> + - items:
Drop Unnecessary properties
> + - enum:
> + - fsl,imx6q-tempmon
> + - fsl,imx6sx-tempmon
> + - fsl,imx7d-tempmon
> +
> + interrupts:
> +description: |
> + The interrupt output of the controller, the IRQ will be triggered
> + when temperature is higher than high threshold.
> +maxItems: 1
You'd better explain why interrupts number is changed in the new binding
compared to
the original one. Probably add in commit message if really needed.
> +
> + nvmem-cells:
> +description: |
> + Phandle to the calibration cells provided by ocotp for calibration
> + data and temperature grade.
Better describe for each of them as you did for clocks
> +maxItems: 2
> +
> + nvmem-cell-names:
> +maxItems: 2
> +items:
> + - const: calib
> + - const: temp_grade
> +
> + fsl,tempmon:
> +$ref: '/schemas/types.yaml#/definitions/phandle'
> +description: Phandle to the register map node.
What register map? A bit ambiguous..
Regards
Aisheng
> +
> + fsl,tempmon-data:
> +$ref: '/schemas/types.yaml#/definitions/phandle'
> +description: |
> + Deprecated property, phandle pointer to fuse controller that contains
> + TEMPMON calibration data, e.g. OCOTP on imx6q. The details about
> + calibration data can be found in SoC Reference Manual.
> +deprecated: true
> +
> + clocks:
> +maxItems: 1
> +
> +required:
> + - compatible
> + - interrupts
> + - fsl,tempmon
> + - nvmem-cells
> + - nvmem-cell-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +
> +efuse@21bc000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,imx6sx-ocotp", "syscon";
> + reg = <0x021bc000 0x4000>;
> + clocks = <&clks IMX6SX_CLK_OCOTP>;
> +
> + tempmon_calib: calib@38 {
> + reg = <0x38 4>;
> + };
> +
> + tempmon_temp_grade: temp-grade@20 {
> + reg = <0x20 4>;
> + };
> +};
> +
> +anatop@20c8000 {
> +compatible = "fsl,imx6q-anatop", "syscon", "simple-mfd";
> +reg = <0x020c8000 0x1000>;
> +interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>,
> + <0 54 IRQ_TYPE_LEVEL_HIGH>,
> + <0 127 IRQ_TYPE_LEVEL_HIGH>;
> +
> +tempmon {
> + compatible = "fsl,imx6sx-tempmon";
> + interrupts = ;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
> +};
> +};
> --
> 2.7.4
RE: [PATCH] ARM: dts: imx: Make tempmon node as child of anatop node
> From: Anson Huang
> Sent: Wednesday, May 20, 2020 2:30 PM
>
> i.MX6/7 SoCs' temperature sensor is inside anatop module from HW perspective,
> so it should be a child node of anatop.
>
> Signed-off-by: Anson Huang
Reviewed-by: Dong Aisheng
BTW, I think you also need a binding doc for this change.
Regards
Aisheng
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 22 +++---
> arch/arm/boot/dts/imx6sl.dtsi | 20 ++--
> arch/arm/boot/dts/imx6sll.dtsi | 20 ++--
> arch/arm/boot/dts/imx6sx.dtsi | 20 ++--
> arch/arm/boot/dts/imx6ul.dtsi | 20 ++--
> arch/arm/boot/dts/imx7s.dtsi | 20 ++--
> 6 files changed, 61 insertions(+), 61 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 39d4afd..43d44d5 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -69,17 +69,6 @@
> };
> };
>
> - tempmon: tempmon {
> - compatible = "fsl,imx6q-tempmon";
> - interrupt-parent = <&gpc>;
> - interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
> - #thermal-sensor-cells = <0>;
> - };
> -
> ldb: ldb {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -795,6 +784,17 @@
> anatop-min-voltage = <725000>;
> anatop-max-voltage = <145>;
> };
> +
> + tempmon: tempmon {
> + compatible = "fsl,imx6q-tempmon";
> + interrupt-parent = <&gpc>;
> + interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib",
> "temp_grade";
> + clocks = <&clks
> IMX6QDL_CLK_PLL3_USB_OTG>;
> + #thermal-sensor-cells = <0>;
> + };
> };
>
> usbphy1: usbphy@20c9000 {
> diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> index 911d8cf..d8efc0a 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -93,16 +93,6 @@
> };
> };
>
> - tempmon: tempmon {
> - compatible = "fsl,imx6q-tempmon";
> - interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-parent = <&gpc>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6SL_CLK_PLL3_USB_OTG>;
> - };
> -
> pmu {
> compatible = "arm,cortex-a9-pmu";
> interrupt-parent = <&gpc>;
> @@ -628,6 +618,16 @@
> anatop-min-voltage = <725000>;
> anatop-max-voltage = <145>;
> };
> +
> + tempmon: tempmon {
> + compatible = "fsl,imx6q-tempmon";
> + interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&gpc>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib",
> "temp_grade";
> + clocks = <&clks
> IMX6SL_CLK_PLL3_USB_OTG>;
> + };
> };
>
> usbphy1: usbphy@20c9000 {
> diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
> index edd3abb..bf7f048 100644
> --- a/arch/arm/boot/dts/imx6sll.dtsi
> +++ b/arch/arm/boot/dts/imx6sll.dtsi
> @@ -105,16 +105,6 @@
> clock-output-names = "ipp_di1";
> };
>
> - tempmon: temperature-sensor {
> - compatible = "fsl,imx6sll-tempmon", "fsl,imx6sx-tempmon";
> - interrupts = ;
> - interrupt-parent = <&gpc>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6SLL_CLK_PLL3_USB_OTG>;
> - };
> -
> soc {
> #address-cells = <1>;
>
RE: [PATCH] ARM: dts: imx: Make tempmon node as child of anatop node
> From: Anson Huang > Sent: Wednesday, May 20, 2020 3:47 PM > > > Subject: RE: [PATCH] ARM: dts: imx: Make tempmon node as child of > > anatop node > > > > > From: Anson Huang > > > Sent: Wednesday, May 20, 2020 2:30 PM > > > > > > i.MX6/7 SoCs' temperature sensor is inside anatop module from HW > > > perspective, so it should be a child node of anatop. > > > > > > Signed-off-by: Anson Huang > > > > Reviewed-by: Dong Aisheng > > > > BTW, I think you also need a binding doc for this change. > > The binding doc is the imx-thermal.yaml I sent out, it is suggested by Rob to > move tempmon into anatop node, that is why I did this patch to align with the > binding doc. That's thermal binding doc. We need a binding doc to describe the constraints for anatop as well. Regards Aisheng > Anson
RE: [PATCH] ARM: dts: imx: Make tempmon node as child of anatop node
> From: Anson Huang > Sent: Wednesday, May 20, 2020 4:12 PM > > > Subject: RE: [PATCH] ARM: dts: imx: Make tempmon node as child of > > anatop node > > > > > From: Anson Huang > > > Sent: Wednesday, May 20, 2020 3:47 PM > > > > > > > Subject: RE: [PATCH] ARM: dts: imx: Make tempmon node as child of > > > > anatop node > > > > > > > > > From: Anson Huang > > > > > Sent: Wednesday, May 20, 2020 2:30 PM > > > > > > > > > > i.MX6/7 SoCs' temperature sensor is inside anatop module from HW > > > > > perspective, so it should be a child node of anatop. > > > > > > > > > > Signed-off-by: Anson Huang > > > > > > > > Reviewed-by: Dong Aisheng > > > > > > > > BTW, I think you also need a binding doc for this change. > > > > > > The binding doc is the imx-thermal.yaml I sent out, it is suggested > > > by Rob to move tempmon into anatop node, that is why I did this > > > patch to align with the binding doc. > > > > That's thermal binding doc. > > We need a binding doc to describe the constraints for anatop as well. > > anatop includes PMU, thermal etc., need to think about how to add it and > where to put it, will think about it later. We can combine them into one. Regards Aisheng > > Anson
RE: [PATCH V3] dt-bindings: thermal: Convert i.MX to json-schema
> From: Anson Huang
> Sent: Wednesday, May 20, 2020 4:17 PM
>
> Convert the i.MX thermal binding to DT schema format using json-schema
>
> Signed-off-by: Anson Huang
> ---
> Changes since V2:
> - remove unnecessary property in compatible;
> - add detail description for interrupts;
> - add description for each item of nvmem-cells;
> - add more detail for "fsl,tempmon" description.
[...]
> +title: NXP i.MX Thermal Binding
> +
> +maintainers:
> + - Shawn Guo
> + - Anson Huang
> +
> +properties:
> + compatible:
> +enum:
> + - fsl,imx6q-tempmon
> + - fsl,imx6sx-tempmon
> + - fsl,imx7d-tempmon
> +
> + interrupts:
> +description: |
> + The interrupt output of the controller, i.MX6Q has IRQ_HIGH which
> + will be triggered when temperature is higher than high threshold,
> + i.MX6SX and i.MX7S/D have two more IRQs than i.MX6Q, one is
> IRQ_LOW
> + and the other is IRQ_PANIC, when temperature is lower than low
> + threshold, IRQ_LOW will be triggered, when temperature is higher
> + than panic threshold, IRQ_PANIC will be triggered, and system can
> + be configured to auto reboot by SRC module for IRQ_PANIC. IRQ_HIGH,
> + IRQ_LOW and IRQ_PANIC share same interrupt output of controller.
> +maxItems: 1
> +
> + nvmem-cells:
> +items:
> + - description: Phandle to the calibration data provided by ocotp
> + - description: Phandle to the temperature grade provided by ocotp
> +
> + nvmem-cell-names:
> +items:
> + - const: calib
> + - const: temp_grade
> +
> + fsl,tempmon:
> +$ref: '/schemas/types.yaml#/definitions/phandle'
> +description: Phandle to the temperature sensor register map node.
This still looks vauge.
How about " Phandle to anatop system controller node"?
Otherwise, you can add my tag:
Reviewed-by: Dong Aisheng
Regards
Aisheng
> +
> + fsl,tempmon-data:
> +$ref: '/schemas/types.yaml#/definitions/phandle'
> +description: |
> + Deprecated property, phandle pointer to fuse controller that contains
> + TEMPMON calibration data, e.g. OCOTP on imx6q. The details about
> + calibration data can be found in SoC Reference Manual.
> +deprecated: true
> +
> + clocks:
> +maxItems: 1
> +
> +required:
> + - compatible
> + - interrupts
> + - fsl,tempmon
> + - nvmem-cells
> + - nvmem-cell-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +
> +efuse@21bc000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,imx6sx-ocotp", "syscon";
> + reg = <0x021bc000 0x4000>;
> + clocks = <&clks IMX6SX_CLK_OCOTP>;
> +
> + tempmon_calib: calib@38 {
> + reg = <0x38 4>;
> + };
> +
> + tempmon_temp_grade: temp-grade@20 {
> + reg = <0x20 4>;
> + };
> +};
> +
> +anatop@20c8000 {
> +compatible = "fsl,imx6q-anatop", "syscon", "simple-mfd";
> +reg = <0x020c8000 0x1000>;
> +interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>,
> + <0 54 IRQ_TYPE_LEVEL_HIGH>,
> + <0 127 IRQ_TYPE_LEVEL_HIGH>;
> +
> +tempmon {
> + compatible = "fsl,imx6sx-tempmon";
> + interrupts = ;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
> +};
> +};
> --
> 2.7.4
RE: [PATCH v2 1/2] arm64: dts: imx8mm-evk: correct ldo1/ldo2 voltage range
> From: Robin Gong > Sent: Thursday, May 21, 2020 10:18 PM > > Correct ldo1 voltage range from wrong high group(3.0v~3.3v) to low group > (1.6v~1.9v) because the ldo1 should be 1.8v. Actually, two voltage groups have > been supported at bd718x7-regulator driver, hence, just corrrect the voltage > range to 1.6v~3.3v. For ldo2@0.8v, correct voltage range too. > Otherwise, ldo1 would be kept @3.0v and ldo2@0.9v which violate i.mx8mm > datasheet as the below warning log in kernel: > > [0.995524] LDO1: Bringing 180uV into 300-300uV > [0.999196] LDO2: Bringing 80uV into 90-90uV > > Signed-off-by: Robin Gong Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH v2 2/2] arm64: dts: imx8mn-ddr4-evk: correct ldo1/ldo2 voltage range
> From: Robin Gong > Sent: Thursday, May 21, 2020 10:18 PM > > Correct ldo1 voltage range from wrong high group(3.0v~3.3v) to low group > (1.6v~1.9v) because the ldo1 should be 1.8v. Actually, two voltage groups have > been supported at bd718x7-regulator driver, hence, just corrrect the voltage > range to 1.6v~3.3v. For ldo2@0.8v, correct voltage range too. > Otherwise, ldo1 would be kept @3.0v and ldo2@0.9v which violate i.mx8mn > datasheet as the below warning log in kernel: > > [0.995524] LDO1: Bringing 180uV into 300-300uV > [0.999196] LDO2: Bringing 80uV into 90-90uV > > Signed-off-by: Robin Gong Reviewed-by: Dong Aisheng Regards Aisheng
RE: [PATCH] firmware: imx: scu-pd: Update comments for single global power domain
Hi Ulf, > From: Ulf Hansson > Sent: Wednesday, March 17, 2021 5:31 PM > > Since the introduction of the PM domain support for the scu-pd, the genpd > framework has been continuously improved. More preciously, using a single > global power domain can quite easily be deployed for imx platforms. > > To avoid confusions, let's therefore make an update to the comments about > the missing pieces. > > Signed-off-by: Ulf Hansson Thanks for the update. Reviewed-by: Dong Aisheng BTW, I'm still uncertain if the new approach can finally work well for i.MX as SCU PD also supports multiple low power state. I could investigate it more when I adding multi low power states support. Regards Aisheng > --- > drivers/firmware/imx/scu-pd.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c > index 08533ee67626..6e178a63442d 100644 > --- a/drivers/firmware/imx/scu-pd.c > +++ b/drivers/firmware/imx/scu-pd.c > @@ -29,6 +29,10 @@ > *The framework needs some proper extension to support multi power > *domain cases. > * > + *Update: Genpd assigns the ->of_node for the virtual device before it > + *invokes ->attach_dev() callback, hence parsing for device resources via > + *DT should work fine. > + * > * 2. It also breaks most of current drivers as the driver probe sequence > *behavior changed if removing ->power_on|off() callback and use > *->start() and ->stop() instead. genpd_dev_pm_attach will only power > @@ -39,8 +43,11 @@ > *domain enabled will trigger a HW access error. That means we need fix > *most drivers probe sequence with proper runtime pm. > * > - * In summary, we need fix above two issue before being able to switch to > - * the "single global power domain" way. > + *Update: Runtime PM support isn't necessary. Instead, this can easily be > + *fixed in drivers by adding a call to dev_pm_domain_start() during > probe. > + * > + * In summary, the second part needs to be addressed via minor updates > + to the > + * relevant drivers, before the "single global power domain" model can be > used. > * > */ > > -- > 2.25.1
RE: [PATCH 01/11] i2c: imx-lpi2c: directly retrun ISR when detect a NACK
> From: Clark Wang
> Sent: Wednesday, March 17, 2021 2:54 PM
>
> A NACK flag in ISR means i2c bus error. In such codition, there is no need to
> do
> read/write operation. It's better to return ISR directly and then stop i2c
> transfer.
>
> Signed-off-by: Gao Pan
> Signed-off-by: Clark Wang
Reviewed-by: Dong Aisheng
Regards
Aisheng
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 9db6ccded5e9..bbf44ac95021 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -507,15 +507,17 @@ static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
> lpi2c_imx_intctrl(lpi2c_imx, 0);
> temp = readl(lpi2c_imx->base + LPI2C_MSR);
>
> + if (temp & MSR_NDF) {
> + complete(&lpi2c_imx->complete);
> + goto ret;
> + }
> +
> if (temp & MSR_RDF)
> lpi2c_imx_read_rxfifo(lpi2c_imx);
> -
> - if (temp & MSR_TDF)
> + else if (temp & MSR_TDF)
> lpi2c_imx_write_txfifo(lpi2c_imx);
>
> - if (temp & MSR_NDF)
> - complete(&lpi2c_imx->complete);
> -
> +ret:
> return IRQ_HANDLED;
> }
>
> --
> 2.25.1
RE: [PATCH 02/11] i2c: imx-lpi2c: add runtime pm support
> From: Clark Wang
> Sent: Wednesday, March 17, 2021 2:54 PM
> Subject: [PATCH 02/11] i2c: imx-lpi2c: add runtime pm support
>
> - Add runtime pm support to dynamicly manage the clock.
> - Put the suspend to suspend_noirq.
> - Call .pm_runtime_force_suspend() to force runtime pm suspended
> in .suspend_noirq().
>
The patch title needs to be improved as the driver already supports rpm.
And do one thing in one patch.
> Signed-off-by: Fugang Duan
> Signed-off-by: Gao Pan
> Reviewed-by: Anson Huang
Please add your sign-off.
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 50 --
> 1 file changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index bbf44ac95021..1e920e7ac7c1 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -574,7 +574,8 @@ static int lpi2c_imx_probe(struct platform_device
> *pdev)
> if (ret)
> lpi2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
>
> - ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> + ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr,
> +IRQF_NO_SUSPEND,
This belongs to a separate patch
> pdev->name, lpi2c_imx);
> if (ret) {
> dev_err(&pdev->dev, "can't claim irq %d\n", irq); @@ -584,35
> +585,32 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
> i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> platform_set_drvdata(pdev, lpi2c_imx);
>
> - ret = clk_prepare_enable(lpi2c_imx->clk);
> - if (ret) {
> - dev_err(&pdev->dev, "clk enable failed %d\n", ret);
> - return ret;
> - }
> -
> pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
> pm_runtime_use_autosuspend(&pdev->dev);
> - pm_runtime_get_noresume(&pdev->dev);
> - pm_runtime_set_active(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> + ret = pm_runtime_get_sync(&pdev->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(&pdev->dev);
> + dev_err(&pdev->dev, "failed to enable clock\n");
> + return ret;
> + }
Can't current clk control via rpm work well?
Please describe why need change.
> +
> temp = readl(lpi2c_imx->base + LPI2C_PARAM);
> lpi2c_imx->txfifosize = 1 << (temp & 0x0f);
> lpi2c_imx->rxfifosize = 1 << ((temp >> 8) & 0x0f);
>
> + pm_runtime_put(&pdev->dev);
> +
> ret = i2c_add_adapter(&lpi2c_imx->adapter);
> if (ret)
> goto rpm_disable;
>
> - pm_runtime_mark_last_busy(&pdev->dev);
> - pm_runtime_put_autosuspend(&pdev->dev);
> -
> dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
>
> return 0;
>
> rpm_disable:
> - pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> pm_runtime_dont_use_autosuspend(&pdev->dev);
>
> @@ -636,7 +634,7 @@ static int __maybe_unused
> lpi2c_runtime_suspend(struct device *dev)
> struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
>
> clk_disable_unprepare(lpi2c_imx->clk);
> - pinctrl_pm_select_sleep_state(dev);
> + pinctrl_pm_select_idle_state(dev);
This belongs to a separate patch
>
> return 0;
> }
> @@ -649,16 +647,34 @@ static int __maybe_unused
> lpi2c_runtime_resume(struct device *dev)
> pinctrl_pm_select_default_state(dev);
> ret = clk_prepare_enable(lpi2c_imx->clk);
> if (ret) {
> - dev_err(dev, "failed to enable I2C clock, ret=%d\n", ret);
> + dev_err(dev, "can't enable I2C clock, ret=%d\n", ret);
Probably unnecessary change
> return ret;
> }
>
> + return ret;
> +}
> +
> +static int lpi2c_suspend_noirq(struct device *dev) {
> + int ret;
> +
> + ret = pm_runtime_force_suspend(dev);
> + if (ret)
> + return ret;
> +
> + pinctrl_pm_select_sleep_state(dev);
> +
> return 0;
> }
>
> +static int lpi2c_resume_noirq(struct device *dev) {
> + return pm_runtime_force_resume(dev);
> +}
> +
> static const struct dev_pm_ops lpi2c_pm_ops = {
> - SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> - pm_runtime_force_resume)
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(lpi2c_suspend_noirq,
> + lpi2c_resume_noirq)
Belongs to separate change and explain why need do this
> SET_RUNTIME_PM_OPS(lpi2c_runtime_suspend,
> lpi2c_runtime_resume, NULL)
> };
> --
> 2.25.1
RE: [PATCH 03/11] i2c: imx-lpi2c: add ipg clk for lpi2c driver
> From: Clark Wang
> Sent: Wednesday, March 17, 2021 2:54 PM
>
> The lpi2c IP needs two clks: ipg clk and per clk. The old lpi2c driver missed
> ipg
> clk. This patch adds ipg clk for lpi2c driver.
>
Pleas also update dt-binding and sent along with this patchset.(before this one)
> Signed-off-by: Gao Pan
> Signed-off-by: Clark Wang
> Acked-by: Fugang Duan
You can drop the Ack tag if the patch was changed
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 28 +---
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 1e920e7ac7c1..664fcc0dba51 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -94,7 +94,8 @@ enum lpi2c_imx_pincfg {
>
> struct lpi2c_imx_struct {
> struct i2c_adapter adapter;
> - struct clk *clk;
> + struct clk *clk_per;
> + struct clk *clk_ipg;
> void __iomem*base;
> __u8*rx_buf;
> __u8*tx_buf;
> @@ -563,10 +564,16 @@ static int lpi2c_imx_probe(struct platform_device
> *pdev)
> strlcpy(lpi2c_imx->adapter.name, pdev->name,
> sizeof(lpi2c_imx->adapter.name));
>
> - lpi2c_imx->clk = devm_clk_get(&pdev->dev, NULL);
> - if (IS_ERR(lpi2c_imx->clk)) {
> + lpi2c_imx->clk_per = devm_clk_get(&pdev->dev, "per");
> + if (IS_ERR(lpi2c_imx->clk_per)) {
> dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
> - return PTR_ERR(lpi2c_imx->clk);
> + return PTR_ERR(lpi2c_imx->clk_per);
> + }
> +
> + lpi2c_imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
> + if (IS_ERR(lpi2c_imx->clk_ipg)) {
> + dev_err(&pdev->dev, "can't get I2C ipg clock\n");
> + return PTR_ERR(lpi2c_imx->clk_ipg);
> }
Will this break exist dts?
Regards
Aisheng
>
> ret = of_property_read_u32(pdev->dev.of_node,
> @@ -633,7 +640,8 @@ static int __maybe_unused
> lpi2c_runtime_suspend(struct device *dev) {
> struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
>
> - clk_disable_unprepare(lpi2c_imx->clk);
> + clk_disable_unprepare(lpi2c_imx->clk_ipg);
> + clk_disable_unprepare(lpi2c_imx->clk_per);
> pinctrl_pm_select_idle_state(dev);
>
> return 0;
> @@ -645,12 +653,18 @@ static int __maybe_unused
> lpi2c_runtime_resume(struct device *dev)
> int ret;
>
> pinctrl_pm_select_default_state(dev);
> - ret = clk_prepare_enable(lpi2c_imx->clk);
> + ret = clk_prepare_enable(lpi2c_imx->clk_per);
> if (ret) {
> - dev_err(dev, "can't enable I2C clock, ret=%d\n", ret);
> + dev_err(dev, "can't enable I2C per clock, ret=%d\n", ret);
> return ret;
> }
>
> + ret = clk_prepare_enable(lpi2c_imx->clk_ipg);
> + if (ret) {
> + clk_disable_unprepare(lpi2c_imx->clk_per);
> + dev_err(dev, "can't enable I2C ipg clock, ret=%d\n", ret);
> + }
> +
> return ret;
> }
>
> --
> 2.25.1
RE: [PATCH 04/11] i2c: imx-lpi2c: manage irq resource request/release in runtime pm
> From: Clark Wang
> Sent: Wednesday, March 17, 2021 2:54 PM
>
> Manage irq resource request/release in runtime pm to save irq domain's
> power.
>
> Signed-off-by: Frank Li
> Signed-off-by: Fugang Duan
> Reviewed-by: Frank Li
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 26 ++
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 664fcc0dba51..e718bb6b2387 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -94,6 +94,7 @@ enum lpi2c_imx_pincfg {
>
> struct lpi2c_imx_struct {
> struct i2c_adapter adapter;
> + int irq;
> struct clk *clk_per;
> struct clk *clk_ipg;
> void __iomem*base;
> @@ -543,7 +544,7 @@ static int lpi2c_imx_probe(struct platform_device
> *pdev) {
> struct lpi2c_imx_struct *lpi2c_imx;
> unsigned int temp;
> - int irq, ret;
> + int ret;
>
> lpi2c_imx = devm_kzalloc(&pdev->dev, sizeof(*lpi2c_imx), GFP_KERNEL);
> if (!lpi2c_imx)
> @@ -553,9 +554,9 @@ static int lpi2c_imx_probe(struct platform_device
> *pdev)
> if (IS_ERR(lpi2c_imx->base))
> return PTR_ERR(lpi2c_imx->base);
>
> - irq = platform_get_irq(pdev, 0);
> - if (irq < 0)
> - return irq;
> + lpi2c_imx->irq = platform_get_irq(pdev, 0);
> + if (lpi2c_imx->irq < 0)
> + return lpi2c_imx->irq;
>
> lpi2c_imx->adapter.owner= THIS_MODULE;
> lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
> @@ -581,14 +582,6 @@ static int lpi2c_imx_probe(struct platform_device
> *pdev)
> if (ret)
> lpi2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
>
> - ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr,
> -IRQF_NO_SUSPEND,
> -pdev->name, lpi2c_imx);
> - if (ret) {
> - dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> - return ret;
> - }
> -
> i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> platform_set_drvdata(pdev, lpi2c_imx);
>
> @@ -640,6 +633,7 @@ static int __maybe_unused
> lpi2c_runtime_suspend(struct device *dev) {
> struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
>
> + devm_free_irq(dev, lpi2c_imx->irq, lpi2c_imx);
> clk_disable_unprepare(lpi2c_imx->clk_ipg);
> clk_disable_unprepare(lpi2c_imx->clk_per);
> pinctrl_pm_select_idle_state(dev);
> @@ -665,6 +659,14 @@ static int __maybe_unused
> lpi2c_runtime_resume(struct device *dev)
> dev_err(dev, "can't enable I2C ipg clock, ret=%d\n", ret);
> }
>
> + ret = devm_request_irq(dev, lpi2c_imx->irq, lpi2c_imx_isr,
I guess unnecessary to use devm in rpm
> +IRQF_NO_SUSPEND,
> +dev_name(dev), lpi2c_imx);
> + if (ret) {
> + dev_err(dev, "can't claim irq %d\n", lpi2c_imx->irq);
> + return ret;
> + }
> +
> return ret;
> }
>
> --
> 2.25.1
RE: [PATCH 05/11] i2c: imx-lpi2c: add debug message when i2c peripheral clk doesn't work
> From: Clark Wang
> Sent: Wednesday, March 17, 2021 2:54 PM
>
> add debug message when i2c peripheral clk rate is 0, then directly return
> -EINVAL.
>
> Signed-off-by: Gao Pan
> Reviewed-by: Andy Duan
Drop old review when patch is changed
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index e718bb6b2387..8f9dd3dd2951 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -209,7 +209,12 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct
> *lpi2c_imx)
>
> lpi2c_imx_set_mode(lpi2c_imx);
>
> - clk_rate = clk_get_rate(lpi2c_imx->clk);
I guess the kernel can't compile right before this patch because lpi2c_imx->clk
was
Removed In former patch
You need double check not break bisect
> + clk_rate = clk_get_rate(lpi2c_imx->clk_per);
> + if (!clk_rate) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "clk_per rate is 0\n");
s/dev_dbg/dev_err
> + return -EINVAL;
> + }
> +
> if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> filt = 0;
> else
> --
> 2.25.1
RE: [PATCH 06/11] i2c: imx-lpi2c: improve i2c driver probe priority
> From: Clark Wang
> Sent: Wednesday, March 17, 2021 2:54 PM
>
> use subsys_initcall for i2c driver to improve i2c driver probe priority
Will this affect DMA support which will be probed much later compared with
subsys_initcall?
>
> Signed-off-by: Gao Pan
Add your sign-off
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 8f9dd3dd2951..86b69852f7be 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -710,7 +710,17 @@ static struct platform_driver lpi2c_imx_driver = {
> },
> };
>
> -module_platform_driver(lpi2c_imx_driver);
> +static int __init lpi2c_imx_init(void)
> +{
> + return platform_driver_register(&lpi2c_imx_driver);
> +}
> +subsys_initcall(lpi2c_imx_init);
> +
> +static void __exit lpi2c_imx_exit(void) {
> + platform_driver_unregister(&lpi2c_imx_driver);
> +}
> +module_exit(lpi2c_imx_exit);
>
> MODULE_AUTHOR("Gao Pan ");
> MODULE_DESCRIPTION("I2C adapter driver for LPI2C bus");
> --
> 2.25.1
