* Re: [PATCH RFC 1/7] pinctrl: sh-pfc: add new flags into struct sh_pfc_pin_config
From: Geert Uytterhoeven @ 2019-08-06 8:46 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <1562576868-8124-2-git-send-email-yoshihiro.shimoda.uh@renesas.com>
On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> To clean/modify the code up later, this patch just adds new flags
> "mux_set" and "gpio_enabled" into the struct sh_pfc_pin_config.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC 2/7] pinctrl: sh-pfc: remove incomplete flag "cfg->type"
From: Geert Uytterhoeven @ 2019-08-06 8:49 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, thierry.reding@gmail.com,
Rob Herring, Mark Rutland, open list:GPIO SUBSYSTEM,
linux-pwm@vger.kernel.org,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <TYAPR01MB4544031C6A027A3690FFFB77D8DD0@TYAPR01MB4544.jpnprd01.prod.outlook.com>
Hi Shimoda-san,
On Mon, Jul 29, 2019 at 7:16 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Linus Walleij, Sent: Monday, July 29, 2019 8:02 AM
> >
> > On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> >
> > > The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> > > support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> > > sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> > > Now if we fix the cfg->type condition, it gets worse because:
> > > - Some drivers might be deferred so that .set_mux() will be called
> > > multiple times.
> > > - In such the case, the sh-pfc driver returns -EBUSY even if
> > > the group is the same, and then that driver fails to probe.
> > >
> > > Since the pinctrl subsystem already has such conditions according
> > > to @set_mux and @gpio_request_enable, this patch just remove
> > > the incomplete flag from sh-pfc/pinctrl.c.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >
> > This looks like it should have a Fixes: tag as well.
>
> I got it. The Fixes tag should be:
>
> Fixes: c58d9c1b26e3 ("sh-pfc: Implement generic pinconf support")
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Geert will decide what to do with this.
>
> I got it.
>
> > Can all the pinctrl patches be applied independently of the other
> > changes so Geert can apply and send me those patches in his pull
> > requests?
>
> The pinctrl patches (1/7 through 3/7) can be applied on next-20190726
> so I think Geert can apply these patches into his repo.
Looks mostly OK to me (I have some comments on 3/7).
I'll apply it to my local tree, so it will receive some testing on all
boards I have.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC 3/7] pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
From: Geert Uytterhoeven @ 2019-08-06 9:02 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <1562576868-8124-4-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> R-Car PWM controller requires the gpio to output zero duty,
> this patch allows to roll it back from gpio to mux when the gpio
> is freed.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Thanks for your patch!
> --- a/drivers/pinctrl/sh-pfc/pinctrl.c
> +++ b/drivers/pinctrl/sh-pfc/pinctrl.c
> @@ -26,6 +26,7 @@
> #include "../pinconf.h"
>
> struct sh_pfc_pin_config {
> + unsigned int mux_mark;
Due to padding, adding this field will increase memory consumption by
6 bytes per pin.
Probably sh_pfc_pin_group.{pins,mux} should be changed from unsigned int
to u16, but that's out of scope for this patch.
> bool mux_set;
> bool gpio_enabled;
> };
> @@ -353,6 +354,15 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> spin_lock_irqsave(&pfc->lock, flags);
>
> for (i = 0; i < grp->nr_pins; ++i) {
> + int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]);
> + struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
> +
> + /*
> + * This doesn't assume the order which gpios are enabled
> + * and then mux is set.
I'm sorry, I don't understand what you mean?
Can you please reword or elaborate?
> + */
> + WARN_ON(cfg->gpio_enabled);
Can this actually happen?
Should this cause a failure instead?
> +
> ret = sh_pfc_config_mux(pfc, grp->mux[i], PINMUX_TYPE_FUNCTION);
> if (ret < 0)
> goto done;
> @@ -364,6 +374,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
>
> cfg->mux_set = true;
> + cfg->mux_mark = grp->mux[i];
> }
>
> done:
> @@ -417,6 +428,9 @@ static void sh_pfc_gpio_disable_free(struct pinctrl_dev *pctldev,
>
> spin_lock_irqsave(&pfc->lock, flags);
> cfg->gpio_enabled = false;
> + /* If mux is already set, this configure it here */
configures
> + if (cfg->mux_set)
> + sh_pfc_config_mux(pfc, cfg->mux_mark, PINMUX_TYPE_FUNCTION);
Have you considered the case where more than one pin of a pinmux group
was used as a GPIO? In that case sh_pfc_gpio_disable_free() will be called
multiple times, possibly with the same mux_mark.
I don't think this will cause issues, though.
> spin_unlock_irqrestore(&pfc->lock, flags);
> }
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC 5/7] pwm: rcar: remove a redundant condition in rcar_pwm_apply()
From: Geert Uytterhoeven @ 2019-08-06 9:05 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <1562576868-8124-6-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Since the rcar_pwm_apply() has already check whehter state->enabled
> is not set or not, this patch removes a redundant condition.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
This is completely independent from the rest of the series, and can be applied
immediately, right?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC 4/7] dt-bindings: pwm: rcar: Add specific gpios property to output duty zero
From: Geert Uytterhoeven @ 2019-08-06 9:21 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <1562576868-8124-5-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> The R-Car SoCs PWM Timer cannot output duty zero. So, this patch
> adds a specific gpio property to output it.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Thanks for your patch!
> --- a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> +++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> @@ -26,6 +26,9 @@ Required Properties:
> - pinctrl-0: phandle, referring to a default pin configuration node.
> - pinctrl-names: Set to "default".
>
> +Optional properties:
> +- renesas,duty-zero-gpios: Specify GPIO for outputting duty zero.
> +
> Example: R8A7743 (RZ/G1M) PWM Timer node
>
> pwm0: pwm@e6e30000 {
I'm not so fond of adding a property to specify this explicitly: the PFC
driver already knows the mapping from the PWM output pin to the GPIO
number. However, I agree it is not easy to obtain this in a generic way.
For a PWM block with a single pin, it's easy: the pin you want to switch
between GPIO and pin function is the single pin in the single pin
control group specified in the board DT.
For blocks with multiple pins (e.g. SPI, UART), it is more complex, and
depends on the granularity of the pin control groups.
E.g. for UART, Renesas SoCs typically use 3 pin control groups ("data"
for RXD/TXD, "ctrl" for RTS/CTS, and "clk" for clock), and the pin
control driver (at least for sh-pfc) does not know which pin corresponds
to which GPIO inside each group. Perhaps this information should be
added, with an API to retrieve it?
Anyone who has a good suggestion?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC 2/7] pinctrl: sh-pfc: remove incomplete flag "cfg->type"
From: Geert Uytterhoeven @ 2019-08-06 9:23 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <1562576868-8124-3-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> Now if we fix the cfg->type condition, it gets worse because:
> - Some drivers might be deferred so that .set_mux() will be called
> multiple times.
> - In such the case, the sh-pfc driver returns -EBUSY even if
> the group is the same, and then that driver fails to probe.
>
> Since the pinctrl subsystem already has such conditions according
> to @set_mux and @gpio_request_enable, this patch just remove
> the incomplete flag from sh-pfc/pinctrl.c.
Do we need to set sh_pfc_pinmux_ops.strict = true?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: imx-ocotp: Add i.MX8MN compatible
From: Srinivas Kandagatla @ 2019-08-06 9:39 UTC (permalink / raw)
To: Anson.Huang, robh+dt, mark.rutland, shawnguo, s.hauer, kernel,
festevam, devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <20190711023714.16000-1-Anson.Huang@nxp.com>
On 11/07/2019 03:37, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
>
> Add compatible for i.MX8MN and add i.MX8MM/i.MX8MN to the description.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Applied both the patches.
Thanks,
srini
> ---
> Documentation/devicetree/bindings/nvmem/imx-ocotp.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt b/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt
> index 96ffd06..904dadf 100644
> --- a/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt
> +++ b/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt
> @@ -2,7 +2,7 @@ Freescale i.MX6 On-Chip OTP Controller (OCOTP) device tree bindings
>
> This binding represents the on-chip eFuse OTP controller found on
> i.MX6Q/D, i.MX6DL/S, i.MX6SL, i.MX6SX, i.MX6UL, i.MX6ULL/ULZ, i.MX6SLL,
> -i.MX7D/S, i.MX7ULP and i.MX8MQ SoCs.
> +i.MX7D/S, i.MX7ULP, i.MX8MQ, i.MX8MM and i.MX8MN SoCs.
>
> Required properties:
> - compatible: should be one of
> @@ -16,6 +16,7 @@ Required properties:
> "fsl,imx7ulp-ocotp" (i.MX7ULP),
> "fsl,imx8mq-ocotp" (i.MX8MQ),
> "fsl,imx8mm-ocotp" (i.MX8MM),
> + "fsl,imx8mn-ocotp" (i.MX8MN),
> followed by "syscon".
> - #address-cells : Should be 1
> - #size-cells : Should be 1
>
^ permalink raw reply
* Re: [RFC,v3 7/9] media: platform: Add Mediatek ISP P1 device driver
From: Tomasz Figa @ 2019-08-06 9:47 UTC (permalink / raw)
To: Jungo Lin
Cc: devicetree, Sean Cheng (鄭昇弘),
Frederic Chen (陳俊元),
Rynn Wu (吳育恩), srv_heupstream, Rob Herring,
Ryan Yu (余孟修),
Frankie Chiu (邱文凱), Hans Verkuil, ddavenport,
Sj Huang, moderated list:ARM/Mediatek SoC support,
Laurent Pinchart, Matthias Brugger, Mauro Carvalho Chehab
In-Reply-To: <1564125828.1212.600.camel@mtksdccf07>
Hi Jungo,
On Fri, Jul 26, 2019 at 4:24 PM Jungo Lin <jungo.lin@mediatek.com> wrote:
>
> Hi, Tomasz:
>
> On Thu, 2019-07-25 at 18:23 +0900, Tomasz Figa wrote:
> > .Hi Jungo,
> >
> > On Sat, Jul 20, 2019 at 6:58 PM Jungo Lin <jungo.lin@mediatek.com> wrote:
> > >
> > > Hi, Tomasz:
> > >
> > > On Wed, 2019-07-10 at 18:56 +0900, Tomasz Figa wrote:
> > > > Hi Jungo,
> > > >
> > > > On Tue, Jun 11, 2019 at 11:53:42AM +0800, Jungo Lin wrote:
[snip]
> > > > > +
> > > > > + err_status = irq_status & INT_ST_MASK_CAM_ERR;
> > > > > +
> > > > > + /* Sof, done order check */
> > > > > + if ((irq_status & SOF_INT_ST) && (irq_status & HW_PASS1_DON_ST)) {
> > > > > + dev_dbg(dev, "sof_done block cnt:%d\n", isp_dev->sof_count);
> > > > > +
> > > > > + /* Notify IRQ event and enqueue frame */
> > > > > + irq_handle_notify_event(isp_dev, irq_status, dma_status, 0);
> > > > > + isp_dev->current_frame = hw_frame_num;
> > > >
> > > > What exactly is hw_frame_num? Shouldn't we assign it before notifying the
> > > > event?
> > > >
> > >
> > > This is a another spare register for frame sequence number usage.
> > > It comes from struct p1_frame_param:frame_seq_no which is sent by
> > > SCP_ISP_FRAME IPI command. We will rename this to dequeue_frame_seq_no.
> > > Is it a better understanding?
> >
> > I'm sorry, unfortunately it's still not clear to me. Is it the
> > sequence number of the frame that was just processed and returned to
> > the kernel or the next frame that is going to be processed from now
> > on?
> >
>
> It is the next frame that is going to be proceed.
> We simplify the implementation of isp_irq_cam function. The hw_frame_num
> is renamed to dequeue_frame_seq_no and saved this value from HW at
> SOF_INT_ST. Since it is obtained in SOF_INI_ST event, it means it is
> next frame to be processed. If there is SW_PASS1_DON_ST, it means this
> frame is processed done. We use this value to de-queue the frame request
> and return buffers to VB2.
>
> The normal IRQ sequence is SOF_INT_ST => SW_PASS1_DON_ST &
> HW_PASS1_DON_ST.
>
> a. SW_PASS_DON_ST is designed for DMAs done event.
> If there is no available DMA buffers en-queued into HW, there is no
> SW_PADD_DON_ST.
>
> b. HW_PASS_DON_ST is designed to trigger CQ buffer load procedure.
> It is paired with SOF IRQ event, even if there is no available DMA
> buffers.
>
> static void isp_irq_handle_sof(struct mtk_isp_p1_device *p1_dev,
> unsigned int dequeue_frame_seq_no)
> {
> dma_addr_t base_addr = p1_dev->composer_iova;
> int composed_frame_seq_no =
> atomic_read(&p1_dev->composed_frame_seq_no);
> unsigned int addr_offset;
>
> /* Send V4L2_EVENT_FRAME_SYNC event */
> mtk_cam_dev_event_frame_sync(&p1_dev->cam_dev, dequeue_frame_seq_no);
>
> p1_dev->sof_count += 1;
> /* Save dequeue frame information */
> p1_dev->dequeue_frame_seq_no = dequeue_frame_seq_no;
>
> /* Update CQ base address if needed */
> if (composed_frame_seq_no <= dequeue_frame_seq_no) {
> dev_dbg(p1_dev->dev,
> "SOF_INT_ST, no update, cq_num:%d, frame_seq:%d",
> composed_frame_seq_no, dequeue_frame_seq_no);
> return;
> }
> addr_offset = MTK_ISP_CQ_ADDRESS_OFFSET *
> (dequeue_frame_seq_no % MTK_ISP_CQ_BUFFER_COUNT);
> writel(base_addr + addr_offset, p1_dev->regs + REG_CQ_THR0_BASEADDR);
> dev_dbg(p1_dev->dev,
> "SOF_INT_ST, update next, cq_num:%d, frame_seq:%d cq_addr:0x%x",
> composed_frame_seq_no, dequeue_frame_seq_no, addr_offset);
> }
>
> void mtk_cam_dev_dequeue_req_frame(struct mtk_cam_dev *cam,
> unsigned int frame_seq_no)
> {
> struct mtk_cam_dev_request *req, *req_prev;
> unsigned long flags;
>
> spin_lock_irqsave(&cam->running_job_lock, flags);
> list_for_each_entry_safe(req, req_prev, &cam->running_job_list, list) {
> dev_dbg(cam->dev, "frame_seq:%d, de-queue frame_seq:%d\n",
> req->frame_params.frame_seq_no, frame_seq_no);
>
> /* Match by the en-queued request number */
> if (req->frame_params.frame_seq_no == frame_seq_no) {
> atomic_dec(&cam->running_job_count);
> /* Pass to user space */
> mtk_cam_dev_job_done(cam, req, VB2_BUF_STATE_DONE);
> list_del(&req->list);
> break;
> } else if (req->frame_params.frame_seq_no < frame_seq_no) {
> atomic_dec(&cam->running_job_count);
> /* Pass to user space for frame drop */
> mtk_cam_dev_job_done(cam, req, VB2_BUF_STATE_ERROR);
> dev_warn(cam->dev, "frame_seq:%d drop\n",
> req->frame_params.frame_seq_no);
> list_del(&req->list);
> } else {
> break;
> }
> }
> spin_unlock_irqrestore(&cam->running_job_lock, flags);
>
> static irqreturn_t isp_irq_cam(int irq, void *data)
> {
> struct mtk_isp_p1_device *p1_dev = (struct mtk_isp_p1_device *)data;
> struct device *dev = p1_dev->dev;
> unsigned int dequeue_frame_seq_no;
> unsigned int irq_status, err_status, dma_status;
> unsigned long flags;
>
> spin_lock_irqsave(&p1_dev->spinlock_irq, flags);
> irq_status = readl(p1_dev->regs + REG_CTL_RAW_INT_STAT);
> err_status = irq_status & INT_ST_MASK_CAM_ERR;
> dma_status = readl(p1_dev->regs + REG_CTL_RAW_INT2_STAT);
> dequeue_frame_seq_no = readl(p1_dev->regs + REG_FRAME_SEQ_NUM);
> spin_unlock_irqrestore(&p1_dev->spinlock_irq, flags);
>
> /*
> * In normal case, the next SOF ISR should come after HW PASS1 DONE
> ISR.
> * If these two ISRs come together, print warning msg to hint.
> */
> if ((irq_status & SOF_INT_ST) && (irq_status & HW_PASS1_DON_ST))
> dev_warn(dev, "sof_done block cnt:%d\n", p1_dev->sof_count);
>
> /* De-queue frame */
> if (irq_status & SW_PASS1_DON_ST) {
> mtk_cam_dev_dequeue_req_frame(&p1_dev->cam_dev,
> dequeue_frame_seq_no);
> mtk_cam_dev_req_try_queue(&p1_dev->cam_dev);
> }
>
> /* Save frame info. & update CQ address for frame HW en-queue */
> if (irq_status & SOF_INT_ST)
> isp_irq_handle_sof(p1_dev, dequeue_frame_seq_no);
>
> /* Check ISP error status */
> if (err_status) {
> dev_err(dev, "int_err:0x%x 0x%x\n", irq_status, err_status);
> /* Show DMA errors in detail */
> if (err_status & DMA_ERR_ST)
> isp_irq_handle_dma_err(p1_dev);
> }
>
> dev_dbg(dev, "SOF:%d irq:0x%x, dma:0x%x, frame_num:%d",
> p1_dev->sof_count, irq_status, dma_status,
> dequeue_frame_seq_no);
>
> return IRQ_HANDLED;
> }
I think I understand this now and the code above also looks good to
me. Thanks a lot!
>
> > >
> > > Below is our frame request handling in current design.
> > >
> > > 1. Buffer preparation
> > > - Combined image buffers (IMGO/RRZO) + meta input buffer (Tuining) +
> > > other meta histogram buffers (LCSO/LMVO) into one request.
> > > - Accumulated one unique frame sequence number to each request and send
> > > this request to the SCP composer to compose CQ (Command queue) buffer
> > > via SCP_ISP_FRAME IPI command.
> > > - CQ buffer is frame registers set. If ISP registers should be updated
> > > per frame, these registers are configured in the CQ buffer, such as
> > > frame sequence number, DMA addresses and tuning ISP registers.
> > > - One frame request will be composed into one CQ buffer.Once CQ buffer
> > > is composed done and kernel driver will receive ISP_CMD_FRAME_ACK with
> > > its corresponding frame sequence number. Based on this, kernel driver
> > > knows which request is ready to be en-queued and save this with
> > > p1_dev->isp_ctx.composed_frame_id.
> >
> > Hmm, why do we need to save this in p1_dev->isp_ctx? Wouldn't we
> > already have a linked lists of requests that are composed and ready to
> > be enqueued? Also, the request itself would contain its frame ID
> > inside the driver request struct, right?
> >
>
> Below is current implementation for frame request en-queued.
> Before en-queued into HW by CQ, the request should be composed by SCP
> composer.
>
> a. mtk_cam_dev_req_try_queue()
> - Insert the request into p1_dev->running_job_list
> b. mtk_isp_req_enqueue()
> - Assign new next frame ID to this request.
> - Sending to SCP by workqueue
> - This request is ready to compose
> c. isp_tx_frame_worker()
> - Send request to SCP with sync. mode. by SCP_IPI_ISP_FRAME command
> - SCP composer will compose the buffer CQ for this request frame based
> on struct mtk_p1_frame_param which includes frame ID.
> - If scp_ipi_send() is returned, it means the request is composed done.
> Or
> d. isp_composer_handler()
> - If we received the ISP_CMD_FRAME_ACK for SCP_IPI_ISP_FRAME, we save
> the frame ID in p1_dev->composed_frame_seq_no which is sent in step C.
> - The request is composed done here.
> e. isp_irq_handle_sof()
> - In SOF timing, we will check there is any available composed CQ
> buffers by comparing composed & current de-queued frame ID.
>
> For p1_dev->running_job_list, we can't guarantee the requests are
> composed until the end of step c. For step e, we need to know how many
> available composed requests are ready to en-queued.
>
> Do you suggest we add another new link-list to save these requests in
> step c or we could update p1_dev->composed_frame_seq_no in step c and
> remove the implementation in step d[1]?
Okay, thanks to your explanation above I think I understood how the
hardware flow behaves and so I think we can indeed keep the
composed_frame_seq counter. Thanks!
>
> [1]. isp_composer_handler() is mandatory callback function for SCP
> sending API with sync mode design.
>
> static void isp_composer_handler(void *data, unsigned int len, void
> *priv)
> {
> struct mtk_isp_p1_device *p1_dev = (struct mtk_isp_p1_device *)priv;
> struct mtk_isp_scp_p1_cmd *ipi_msg;
>
> ipi_msg = (struct mtk_isp_scp_p1_cmd *)data;
>
> if (ipi_msg->cmd_id != ISP_CMD_ACK)
> return;
>
> if (ipi_msg->ack_info.cmd_id == ISP_CMD_FRAME_ACK) {
> atomic_set(&p1_dev->composed_frame_seq_no,
> ipi_msg->ack_info.frame_seq_no);
> dev_dbg(p1_dev->dev, "ack frame_num:%d\n",
> p1_dev->composed_frame_seq_no);
> }
> }
>
> > > - The maximum number of CQ buffers in SCP is 3.
> > >
> > > 2. Buffer en-queue flow
> > > - In order to configure correct CQ buffer setting before next SQF event,
> > > it is depended on by MTK ISP P1 HW CQ mechanism.
> > > - The basic concept of CQ mechanism is loaded ISP CQ buffer settings
> > > when HW_PASS1_DON_ST is received which means DMA output is done.
> > > - Btw, the pre-condition of this, need to tell ISP HW which CQ buffer
> > > address is used. Otherwise, it will loaded one dummy CQ buffer to
> > > bypass.
> > > - So we will check available CQ buffers by comparing composed frame
> > > sequence number & dequeued frame sequence from ISP HW in SOF event.
> > > - If there are available CQ buffers, update the CQ base address to the
> > > next CQ buffer address based on current de-enqueue frame sequence
> > > number. So MTK ISP P1 HW will load this CQ buffer into HW when
> > > HW_PASS1_DON_ST is triggered which is before the next SOF.
> > > - So in next SOF event, ISP HW starts to output DMA buffers with this
> > > request until request is done.
> > > - But, for the first request, it is loaded into HW manually when
> > > streaming is on for better performance.
> > >
> > > 3. Buffer de-queue flow
> > > - We will use frame sequence number to decide which request is ready to
> > > de-queue.
> > > - We will save some important register setting from ISP HW when SOF is
> > > received. This is because the ISP HW starts to output the data with the
> > > corresponding settings, especially frame sequence number setting.
> >
> > Could you explain a bit more about these important register settings?
> > When does the hardware update the values in the register to new ones?
> > At SOF?
> >
>
> Sorry about my words.
> In the current implementation, we just save frame ID.
>
Ah, okay, makes sense. No worries. :)
>
> > > - When receiving SW_PASS1_DON_ST IRQ event, it means the DMA output is
> > > done. So we could call isp_deque_request_frame with frame sequence
> > > number to de-queue frame to VB2
> >
> > What's the difference between HW_PASS1_DON_ST and SW_PASS1_DON_ST?
> >
>
> This is explained above.
>
> > > - For AAO/AFO buffers, it has similar design concept. Sometimes, if only
> > > AAO/AFO non-request buffers are en-queued without request buffers at the
> > > same time, there will be no SW P1 done event for AAO/AFO DMA done.
> > > Needs to depend on other IRQ events, such as AAO/AFO_DONE_EVENT.
> >
> > Do we have a case like this? Wouldn't we normally always want to
> > bundle AAO/AFO buffers with frame buffers?
> >
>
> For upstream driver, we will remove non-request design.
>
I think we also talked about a thing related to this in the thread for
another patch from this series. Basically on Chrome OS we want to use
the upstream driver, so corresponding userspace changes might be
needed as well.
> > > - Due to CQ buffer number limitation, if we receive SW_PASS1_DONT_ST,
> > > we may try to send another request to SCP for composing.
> >
> > Okay, so basically in SW_PASS1_DONT_ST the CQ completed reading the CQ
> > buffers, right?
> >
>
> We expected the the life cycle of CQ buffer is same as frame request.
> So SW_PASS1_DON_ST is good timing to re-queue the next request to
> compose.
> For the CQ operations, we will explain later.
>
> > >
> > > Hopefully, my explanation is helpful for better understanding our
> > > implementation. If you still have any questions, please let me know.
> > >
> >
> > Yes, it's more clear now, thanks. Still some more comments above, though.
> >
> > > > > + isp_dev->meta0_vb2_index = meta0_vb2_index;
> > > > > + isp_dev->meta1_vb2_index = meta1_vb2_index;
> > > > > + } else {
> > > > > + if (irq_status & SOF_INT_ST) {
> > > > > + isp_dev->current_frame = hw_frame_num;
> > > > > + isp_dev->meta0_vb2_index = meta0_vb2_index;
> > > > > + isp_dev->meta1_vb2_index = meta1_vb2_index;
> > > > > + }
> > > > > + irq_handle_notify_event(isp_dev, irq_status, dma_status, 1);
> > > > > + }
> > > >
> > > > The if and else blocks do almost the same things just in different order. Is
> > > > it really expected?
> > > >
> > >
> > > If we receive HW_PASS1_DON_ST & SOF_INT_ST IRQ events at the same time,
> > > the correct sequence should be handle HW_PASS1_DON_ST firstly to check
> > > any de-queued frame and update the next frame setting later.
> > > Normally, this is a corner case or system performance issue.
> >
> > So it sounds like HW_PASS1_DON_ST means that all data from current
> > frame has been written, right? If I understand your explanation above
> > correctly, that would mean following handling of each interrupt:
> >
> > HW_PASS1_DON_ST:
> > - CQ executes with next CQ buffer to prepare for next frame. <- how
> > is this handled? does the CQ hardware automatically receive this event
> > from the ISP hadware?
> > - return VB2 buffers,
> > - complete requests.
> >
> > SOF_INT_ST:
> > - send VSYNC event to userspace,
> > - program next CQ buffer to CQ,
> >
> > SW_PASS1_DON_ST:
> > - reclaim CQ buffer and enqueue next frame to composing if available
> >
>
> Sorry for our implementation of HW_PASS1_DON_ST.
> It is confusing.
> Below is the revised version based on your conclusion.
> So in our new implemmenation, we just handle SOF_INT_ST &
> SW_PASS1_DON_ST events. We just add one warning message for
> HW_PASS1_DON_ST
>
> HW_PASS1_DON_ST:
> - CQ executes with next CQ buffer to prepare for next frame.
>
> SOF_INT_ST:
> - send VSYNC event to userspace,
> - program next CQ buffer to CQ,
>
> SW_PASS1_DON_ST:
> - reclaim CQ buffer and enqueue next frame to composing if available
> - return VB2 buffers,
> - complete requests.
>
> For CQ HW operations, it is listed below:
>
> a. The CQ buffer has two kinds of information
> - Which ISP registers needs to be updated.
> - Where the corresponding ISP register data to be read.
> b. The CQ buffer loading procedure is triggered by HW_PASS1_DONT_ST IRQ
> event periodically.
> - Normally, if the ISP HW receives the completed frame and it will
> trigger W_PASS1_DONT_ST IRQ and perform CQ buffer loading immediately.
> - So the CQ buffer loading is performed by ISP HW automatically.
> c. The ISP HW will read CQ base address register(REG_CQ_THR0_BASEADDR)
> to decide which CQ buffer is loaded.
> - So we configure the next CQ base address in SOF.
> d. For CQ buffer loading, CQ will read the ISP registers from CQ buffer
> and update the ISP register values into HW.
> - SCP composer will compose one dummy CQ buffer and assign it to
> REG_CQ_THR0_BASEADDR of each CQ buffer.
> - Dummy CQ buffer has no updated ISP registers comparing with other
> CQ buffers.
> - With this design, if there is no updated new CQ buffer by driver
> which may be caused no en-queue frames from user space. The CQ HW will
> load dummy CQ buffer and do nothing.
Does the set of registers programmed by CQ include destination buffer
addresses to? If yes, we would end up overwriting previous frames if
no new buffers are provided.
> f. The CQ buffer loading is guaranteed by HW to finish before the next
> SOF.
>
Okay, thanks a lot for the explanation. This is much more clear now.
[snip]
> > > > > +static const struct dev_pm_ops mtk_isp_pm_ops = {
> > > > > + SET_SYSTEM_SLEEP_PM_OPS(mtk_isp_suspend, mtk_isp_resume)
> > > > > + SET_RUNTIME_PM_OPS(mtk_isp_suspend, mtk_isp_resume, NULL)
> > > >
> > > > For V4L2 drivers system and runtime PM ops would normally be completely
> > > > different. Runtime PM ops would be called when the hardware is idle already
> > > > or is about to become active. System PM ops would be called at system power
> > > > state change and the hardware might be both idle or active. Please also see
> > > > my comments to mtk_isp_suspend() and mtk_isp_resume() above.
> > > >
> > >
> > > Here is the new implementation. It should be clear to show the
> > > difference between system and runtime PM ops.
> > >
> > > static const struct dev_pm_ops mtk_isp_pm_ops = {
> > > SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > > pm_runtime_force_resume)
> > > SET_RUNTIME_PM_OPS(mtk_isp_runtime_suspend, mtk_isp_runtime_resume,
> > > NULL)
> > > };
> >
> > That's still not correct. In runtime suspend/resume ops we already are
> > not streaming anymore, because we call pm_runtime_get/put_*() when
> > starting and stopping streaming. In system suspend/resume ops we might
> > be streaming and that's when we need to stop the hardware and wait for
> > it to finish. Please implement these ops separately.
> >
> > Best regards,
> > Tomasz
>
>
> Ok, got your point.
> Below is the new implementation for your review.
>
> static int mtk_isp_pm_suspend(struct device *dev)
> {
> struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(dev);
> u32 val;
> int ret;
>
> dev_dbg(dev, "- %s\n", __func__);
>
> /* Check ISP is streaming or not */
> if (!p1_dev->cam_dev.streaming)
> goto done;
We would normally check here for pm_runtime_suspended(). Although they
both should be equivalent. Still, there is no need to call
pm_runtime_force_suspend() if the latter is true, so we could just
return 0 instantly.
>
> /* Disable ISP's view finder and wait for TG idle */
> dev_dbg(dev, "Cam suspend, disable VF\n");
> val = readl(p1_dev->regs + REG_TG_VF_CON);
> writel(val & (~TG_VF_CON_VFDATA_EN), p1_dev->regs + REG_TG_VF_CON);
> ret = readl_poll_timeout_atomic(p1_dev->regs + REG_TG_INTER_ST, val,
> (val & TG_CS_MASK) == TG_IDLE_ST,
> USEC_PER_MSEC, MTK_ISP_STOP_HW_TIMEOUT);
> if (ret)
> dev_warn(dev, "can't stop HW:%d:0x%x\n", ret, val);
>
> /* Disable CMOS */
> val = readl(p1_dev->regs + REG_TG_SEN_MODE);
> writel(val & (~TG_SEN_MODE_CMOS_EN), p1_dev->regs + REG_TG_SEN_MODE);
>
> done:
> /* Force ISP HW to idle */
> ret = pm_runtime_force_suspend(dev);
> if (ret)
> return ret;
>
> return 0;
> }
>
> static int mtk_isp_pm_resume(struct device *dev)
> {
> struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(dev);
> u32 val;
> int ret;
>
> dev_dbg(dev, "- %s\n", __func__);
>
> /* Force ISP HW to resume if needed */
> ret = pm_runtime_force_resume(dev);
> if (ret)
> return ret;
We should do this conditionally based on what pm_runtime_suspended()
returns. If it's non-zero then we can just return 0 instantly.
>
> if (!p1_dev->cam_dev.streaming)
> return 0;
>
> /* Enable CMOS */
> dev_dbg(dev, "Cam resume, enable CMOS/VF\n");
> val = readl(p1_dev->regs + REG_TG_SEN_MODE);
> writel(val | TG_SEN_MODE_CMOS_EN, p1_dev->regs + REG_TG_SEN_MODE);
>
> /* Enable VF */
> val = readl(p1_dev->regs + REG_TG_VF_CON);
> writel(val | TG_VF_CON_VFDATA_EN, p1_dev->regs + REG_TG_VF_CON);
>
> return 0;
> }
>
> static int mtk_isp_runtime_suspend(struct device *dev)
> {
> struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(dev);
>
> dev_dbg(dev, "- %s\n", __func__);
>
> if (pm_runtime_suspended(dev))
> return 0;
Sorry, I guess I wasn't clear in my reply. It's not possible to get
this callback called if the device is already runtime suspended.
>
> dev_dbg(dev, "%s:disable clock\n", __func__);
> clk_bulk_disable_unprepare(p1_dev->num_clks, p1_dev->clks);
>
> return 0;
> }
>
> static int mtk_isp_runtime_resume(struct device *dev)
> {
> struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(dev);
> int ret;
>
> dev_dbg(dev, "- %s\n", __func__);
>
> if (pm_runtime_suspended(dev))
> return 0;
In this case the above call would always return non-zero, so the
behavior wouldn't be very good.
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: amazon: add Amazon Annapurna Labs Alpine v3 support
From: Krupnik @ 2019-08-06 10:02 UTC (permalink / raw)
To: Sudeep Holla
Cc: robh+dt, mark.rutland, devicetree, linux-kernel, barakw, dwmw,
benh, jonnyc, talel, hhhawa, hanochu
In-Reply-To: <20190729101250.GA831@e107155-lin>
On 7/29/19 13:12, Sudeep Holla wrote:
> On Sun, Jul 28, 2019 at 10:51:35PM +0300, Ronen Krupnik wrote:
>> This patch adds the initial support for the Amazon Annapurna Labs Alpine v3
>> Soc and Evaluation Platform (EVP).
> [...]
>
>> +
>> + pmu {
>> + compatible = "arm,armv8-pmuv3";
> Please use "arm,cortex-a72-pmu" as we know it's Cortex-A72 cores
ack. will be part of v2
>
> --
> Regards,
> Sudeep
^ permalink raw reply
* Re: [PATCH V5 1/5] clocksource: imx-sysctr: Add internal clock divider handle
From: Daniel Lezcano @ 2019-08-06 10:27 UTC (permalink / raw)
To: Anson Huang, catalin.marinas@arm.com, will@kernel.org,
robh+dt@kernel.org, mark.rutland@arm.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
dl-linux-imx, tglx@linutronix.de, Leonard Crestez, Aisheng Dong,
Daniel Baluta, Jacky Bai, l.stach@pengutronix.de, Abel Vesa,
andrew.smirnov@gmail.com, ccaione@baylibre.com, angus@akkea.ca,
agx@sigxcpu.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <DB3PR0402MB3916B06E8907604A71169063F5D50@DB3PR0402MB3916.eurprd04.prod.outlook.com>
On 06/08/2019 03:55, Anson Huang wrote:
> Gentle ping...
Coming back from vacation. It is in the pipe ... :)
>> From: Anson Huang <Anson.Huang@nxp.com>
>>
>> The system counter block guide states that the base clock is internally divided
>> by 3 before use, that means the clock input of system counter defined in DT
>> should be base clock which is normally from OSC, and then internally divided
>> by 3 before use.
>>
>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>> ---
>> Changes since V4:
>> - to solve the clock driver probed after system counter driver issue,
>> now we can easily switch to
>> use fixed clock defined in DT and get its rate, then divided by 3 to
>> get real clock rate for
>> system counter driver, no need to add "clock-frequency" property in
>> DT.
>> ---
>> drivers/clocksource/timer-imx-sysctr.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/clocksource/timer-imx-sysctr.c
>> b/drivers/clocksource/timer-imx-sysctr.c
>> index fd7d680..b7c80a3 100644
>> --- a/drivers/clocksource/timer-imx-sysctr.c
>> +++ b/drivers/clocksource/timer-imx-sysctr.c
>> @@ -20,6 +20,8 @@
>> #define SYS_CTR_EN 0x1
>> #define SYS_CTR_IRQ_MASK 0x2
>>
>> +#define SYS_CTR_CLK_DIV 0x3
>> +
>> static void __iomem *sys_ctr_base;
>> static u32 cmpcr;
>>
>> @@ -134,6 +136,9 @@ static int __init sysctr_timer_init(struct device_node
>> *np)
>> if (ret)
>> return ret;
>>
>> + /* system counter clock is divided by 3 internally */
>> + to_sysctr.of_clk.rate /= SYS_CTR_CLK_DIV;
>> +
>> sys_ctr_base = timer_of_base(&to_sysctr);
>> cmpcr = readl(sys_ctr_base + CMPCR);
>> cmpcr &= ~SYS_CTR_EN;
>> --
>> 2.7.4
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [RFCv3 0/3] interconnect: Add imx support via devfreq
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
To: Georgi Djakov, Rob Herring, Artur Świgoń,
Alexandre Bailon, Viresh Kumar
Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
linux-arm-kernel
This series add imx support for interconnect via devfreq: the ICC
framework is used to aggregate requests from devices and then those are
converted to "min_freq" requests to devfreq.
The dev_pm_qos API is used to request min frequencies from devfreq, it
relies on this patch: https://patchwork.kernel.org/patch/11078475/
This is greatly reduced compared to previous submissions:
* Relying on devfreq and dev_pm_qos instead of CLK allows a wider range
of implementations for scaling individual nodes. In particular it's no
longer required to force DDR scaling into CLK.
* No more "platform opp" stuff: forcing everything to scale together
loses many of the benefits of ICC. The devfreq "passive" governor can
still be used to tie some pieces of the fabric together.
* No more suspend/resume handling: devfreq uses OPP framework which
already supports "suspend-opp".
* Replace all mentions of "busfreq" with "interconnect"
Link to v2: https://patchwork.kernel.org/patch/11056789/
The per-soc platform drivers still need to defined the interconnect
graph and links to devfreq (by name) as well as bus widths. For example
some of the 128-bit buses on imx8m mini are 64-bit on imx8m nano.
There was another recent series tying icc and devfreq, for exynos:
* https://lkml.org/lkml/2019/7/23/394
That series defines the interconnect graph/tree entirely inside
devicetree but that seems limiting. I'd rather keep the graph in a SOC
driver and #define ids for all bus masters/slaves. This way if USB1 and
USB2 are on the same bus they can still be treated differently.
One interesting idea from there is to turn devfreq nodes into
interconnect providers, this can avoid the need for a "virtual" ICC node
in devicetree. I expect devicetree folks will object to that?
Leonard Crestez (3):
dt-bindings: interconnect: Add bindings for imx
interconnect: Add imx core driver
interconnect: imx: Add platform driver for imx8mm
.../devicetree/bindings/interconnect/imx.yaml | 38 +++
drivers/interconnect/Kconfig | 1 +
drivers/interconnect/Makefile | 1 +
drivers/interconnect/imx/Kconfig | 9 +
drivers/interconnect/imx/Makefile | 2 +
drivers/interconnect/imx/imx.c | 258 ++++++++++++++++++
drivers/interconnect/imx/imx.h | 62 +++++
drivers/interconnect/imx/imx8mm.c | 114 ++++++++
include/dt-bindings/interconnect/imx8mm.h | 49 ++++
9 files changed, 534 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
create mode 100644 drivers/interconnect/imx/Kconfig
create mode 100644 drivers/interconnect/imx/Makefile
create mode 100644 drivers/interconnect/imx/imx.c
create mode 100644 drivers/interconnect/imx/imx.h
create mode 100644 drivers/interconnect/imx/imx8mm.c
create mode 100644 include/dt-bindings/interconnect/imx8mm.h
--
2.17.1
^ permalink raw reply
* [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
To: Georgi Djakov, Rob Herring, Artur Świgoń,
Alexandre Bailon, Viresh Kumar
Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
linux-arm-kernel
In-Reply-To: <cover.1565088423.git.leonard.crestez@nxp.com>
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
.../devicetree/bindings/interconnect/imx.yaml | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
new file mode 100644
index 000000000000..c6f173b38f4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/imx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic i.MX interconnect device
+
+maintainers:
+ - Leonard Crestez <leonard.crestez@nxp.com>
+
+properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8mm-interconnect
+ "#interconnect-cells":
+ const: 1
+ devfreq-names:
+ description: Names of devfreq instances for adjustable nodes
+ devfreq:
+ description: List of phandle pointing to devfreq instances
+
+required:
+ - compatible
+ - "#interconnect-cells"
+ - "devfreq-names"
+ - "devfreq"
+
+examples:
+ - |
+ #include <dt-bindings/interconnect/imx8mm.h>
+ icc: interconnect {
+ compatible = "fsl,imx8mm-interconnect";
+ #interconnect-cells = <1>;
+ devfreq-names = "dram", "noc", "axi";
+ devfreq = <&ddrc>, <&noc>, <&pl301_main>;
+ };
--
2.17.1
^ permalink raw reply related
* [RFCv3 2/3] interconnect: Add imx core driver
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
To: Georgi Djakov, Rob Herring, Artur Świgoń,
Alexandre Bailon, Viresh Kumar
Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
linux-arm-kernel
In-Reply-To: <cover.1565088423.git.leonard.crestez@nxp.com>
This adds support for i.MX SoC family to interconnect framework.
Platform drivers can describe their interconnect graph and
several adjustment knobs where an icc node bandwith converted to a
clk_min_rate request.
All adjustable nodes are assumed to be independent.
Based on an earlier work by Alexandre Bailon but greatly reduced to drop
"platform opp" support.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
drivers/interconnect/Kconfig | 1 +
drivers/interconnect/Makefile | 1 +
drivers/interconnect/imx/Kconfig | 5 +
drivers/interconnect/imx/Makefile | 1 +
drivers/interconnect/imx/imx.c | 258 ++++++++++++++++++++++++++++++
drivers/interconnect/imx/imx.h | 62 +++++++
6 files changed, 328 insertions(+)
create mode 100644 drivers/interconnect/imx/Kconfig
create mode 100644 drivers/interconnect/imx/Makefile
create mode 100644 drivers/interconnect/imx/imx.c
create mode 100644 drivers/interconnect/imx/imx.h
diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
index bfa4ca3ab7a9..e61802230f90 100644
--- a/drivers/interconnect/Kconfig
+++ b/drivers/interconnect/Kconfig
@@ -10,7 +10,8 @@ menuconfig INTERCONNECT
If unsure, say no.
if INTERCONNECT
source "drivers/interconnect/qcom/Kconfig"
+source "drivers/interconnect/imx/Kconfig"
endif
diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
index 28f2ab0824d5..20a13b7eb37f 100644
--- a/drivers/interconnect/Makefile
+++ b/drivers/interconnect/Makefile
@@ -2,5 +2,6 @@
icc-core-objs := core.o
obj-$(CONFIG_INTERCONNECT) += icc-core.o
obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/
+obj-$(CONFIG_INTERCONNECT_IMX) += imx/
diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
new file mode 100644
index 000000000000..45fbae7007af
--- /dev/null
+++ b/drivers/interconnect/imx/Kconfig
@@ -0,0 +1,5 @@
+config INTERCONNECT_IMX
+ bool "i.MX interconnect drivers"
+ depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
+ help
+ Generic interconnect driver for i.MX SOCs
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
new file mode 100644
index 000000000000..bb92fd9fe4a5
--- /dev/null
+++ b/drivers/interconnect/imx/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
new file mode 100644
index 000000000000..cc838e40419e
--- /dev/null
+++ b/drivers/interconnect/imx/imx.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/interconnect-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+#include <linux/devfreq.h>
+#include <linux/of.h>
+
+#include "imx.h"
+
+/* private icc_provider data */
+struct imx_icc_provider {
+ struct device *dev;
+};
+
+/* private icc_node data */
+struct imx_icc_node {
+ const struct imx_icc_node_desc *desc;
+ struct devfreq *devfreq;
+ struct dev_pm_qos_request qos_req;
+};
+
+static int imx_icc_aggregate(struct icc_node *node, u32 avg_bw,
+ u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+ *agg_avg += avg_bw;
+ *agg_peak = max(*agg_peak, peak_bw);
+
+ return 0;
+}
+
+static struct icc_node* imx_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+ struct imx_icc_provider *desc = data;
+ struct icc_provider *provider = dev_get_drvdata(desc->dev);
+ unsigned int id = spec->args[0];
+ struct icc_node *node;
+
+ list_for_each_entry (node, &provider->nodes, node_list)
+ if (node->id == id)
+ return node;
+
+ return ERR_PTR(-EINVAL);
+}
+
+static int imx_icc_node_set(struct icc_node *node)
+{
+ struct device *dev = node->provider->dev;
+ struct imx_icc_node *node_data = node->data;
+ unsigned long freq;
+
+ if (!node_data->devfreq)
+ return 0;
+
+ freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
+ do_div(freq, node_data->desc->adj->bw_div);
+ if (freq > INT_MAX) {
+ dev_err(dev, "%s can't request more INT_MAX freq\n",
+ node->name);
+ return -ERANGE;
+ }
+
+ dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
+ node->name, node->avg_bw, node->peak_bw, freq);
+
+ dev_pm_qos_update_request(&node_data->qos_req, freq);
+
+ return 0;
+}
+
+static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ return imx_icc_node_set(dst);
+}
+
+static int imx_icc_node_init_devfreq(struct device *dev,
+ struct icc_node *node)
+{
+ struct imx_icc_node *node_data = node->data;
+ const struct imx_icc_node_desc *node_desc = node_data->desc;
+ int index;
+ int ret;
+
+ index = of_property_match_string(dev->of_node,
+ "devfreq-names", node_desc->adj->devfreq_name);
+ if (index < 0) {
+ dev_err(dev, "failed to match devfreq-names %s: %d\n",
+ node_desc->adj->devfreq_name, index);
+ return index;
+ }
+
+ node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
+ if (IS_ERR(node_data->devfreq)) {
+ ret = PTR_ERR(node_data->devfreq);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
+ index, node_desc->adj->devfreq_name, ret);
+ return ret;
+ }
+
+ return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
+ &node_data->qos_req,
+ DEV_PM_QOS_MIN_FREQUENCY, 0);
+}
+
+static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
+ const struct imx_icc_node_desc *node_desc)
+{
+ struct imx_icc_provider *provider_data = provider->data;
+ struct device *dev = provider_data->dev;
+ struct imx_icc_node *node_data;
+ struct icc_node *node;
+ int ret;
+
+ node = icc_node_create(node_desc->id);
+ if (IS_ERR(node)) {
+ dev_err(dev, "failed to create node %d\n", node_desc->id);
+ return node;
+ }
+
+ if (node->data) {
+ dev_err(dev, "already created node %s id=%d\n",
+ node_desc->name, node_desc->id);
+ return ERR_PTR(-EEXIST);
+ }
+
+ node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
+ if (!node_data) {
+ icc_node_destroy(node->id);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ node->name = node_desc->name;
+ node->data = node_data;
+ node_data->desc = node_desc;
+ if (node_desc->adj) {
+ ret = imx_icc_node_init_devfreq(dev, node);
+ if (ret < 0) {
+ icc_node_destroy(node->id);
+ return ERR_PTR(ret);
+ }
+ }
+
+ icc_node_add(node, provider);
+
+ return node;
+}
+
+static void imx_icc_unregister_nodes(struct icc_provider *provider)
+{
+ struct icc_node *node, *tmp;
+
+ list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
+ struct imx_icc_node *node_data = node->data;
+
+ icc_node_del(node);
+ icc_node_destroy(node->id);
+ if (dev_pm_qos_request_active(&node_data->qos_req))
+ dev_pm_qos_remove_request(&node_data->qos_req);
+ }
+}
+
+static int imx_icc_register_nodes(struct icc_provider *provider,
+ const struct imx_icc_node_desc *descs,
+ int count)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < count; i++) {
+ struct icc_node *node;
+ const struct imx_icc_node_desc *node_desc = &descs[i];
+ size_t j;
+
+ node = imx_icc_node_add(provider, node_desc);
+ if (IS_ERR(node)) {
+ ret = PTR_ERR(node);
+ if (ret != -EPROBE_DEFER)
+ dev_err(provider->dev, "failed to add %s: %d\n",
+ node_desc->name, ret);
+ goto err;
+ }
+
+ for (j = 0; j < node_desc->num_links; j++)
+ icc_link_create(node, node_desc->links[j]);
+ }
+
+ return 0;
+
+err:
+ imx_icc_unregister_nodes(provider);
+
+ return ret;
+}
+
+int imx_icc_register(struct platform_device *pdev,
+ struct imx_icc_node_desc *nodes, int nodes_count)
+{
+ struct device *dev = &pdev->dev;
+ struct imx_icc_provider *desc;
+ struct icc_provider *provider;
+ int ret;
+
+ desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+ if (!desc)
+ return -ENOMEM;
+ desc->dev = dev;
+
+ provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
+ if (!provider)
+ return -ENOMEM;
+ provider->set = imx_icc_set;
+ provider->aggregate = imx_icc_aggregate;
+ provider->xlate = imx_icc_xlate;
+ provider->data = desc;
+ provider->dev = dev;
+ platform_set_drvdata(pdev, provider);
+
+ ret = icc_provider_add(provider);
+ if (ret) {
+ dev_err(dev, "error adding interconnect provider\n");
+ return ret;
+ }
+
+ ret = imx_icc_register_nodes(provider, nodes, nodes_count);
+ if (ret) {
+ dev_err(dev, "error adding interconnect nodes\n");
+ goto provider_del;
+ }
+
+ return 0;
+
+provider_del:
+ icc_provider_del(provider);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(imx_icc_register);
+
+int imx_icc_unregister(struct platform_device *pdev)
+{
+ struct icc_provider *provider = platform_get_drvdata(pdev);
+
+ icc_provider_del(provider);
+ imx_icc_unregister_nodes(provider);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(imx_icc_unregister);
diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
new file mode 100644
index 000000000000..ab191eb89616
--- /dev/null
+++ b/drivers/interconnect/imx/imx.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+#ifndef __BUSFREQ_H
+#define __BUSFREQ_H
+
+#include <linux/kernel.h>
+
+#define IMX_ICC_MAX_LINKS 32
+#define IMX_ICC_UNDEFINED_BW 0xffffffff
+
+/*
+ * struct imx_icc_node_adj - Describe an dynamic adjustment knob
+ */
+struct imx_icc_node_adj_desc {
+ const char *devfreq_name;
+ unsigned int bw_mul, bw_div;
+};
+
+/*
+ * struct imx_icc_node - Describe an interconnect node
+ * @name: name of the node
+ * @id: an unique id to identify the node
+ * @links: an array of slaves' node id
+ * @num_links: number of id defined in links
+ */
+struct imx_icc_node_desc {
+ const char *name;
+ u16 id;
+ u16 links[IMX_ICC_MAX_LINKS];
+ u16 num_links;
+
+ const struct imx_icc_node_adj_desc *adj;
+};
+
+#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...) \
+ { \
+ .id = _id, \
+ .name = _name, \
+ .adj = _adj, \
+ .num_links = _numlinks, \
+ .links = { __VA_ARGS__ }, \
+ }
+
+#define DEFINE_BUS_MASTER(_name, _id, _dest_id) \
+ DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
+
+#define DEFINE_BUS_SLAVE(_name, _id, _adj) \
+ DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
+
+int imx_icc_register(struct platform_device *pdev,
+ struct imx_icc_node_desc *nodes,
+ int nodes_count);
+int imx_icc_unregister(struct platform_device *pdev);
+
+#endif /* __BUSFREQ_H */
--
2.17.1
^ permalink raw reply related
* [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
To: Georgi Djakov, Rob Herring, Artur Świgoń,
Alexandre Bailon, Viresh Kumar
Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
linux-arm-kernel
In-Reply-To: <cover.1565088423.git.leonard.crestez@nxp.com>
This adds a platform driver for the i.MX8MM SoC.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
drivers/interconnect/imx/Kconfig | 4 +
drivers/interconnect/imx/Makefile | 1 +
drivers/interconnect/imx/imx8mm.c | 114 ++++++++++++++++++++++
include/dt-bindings/interconnect/imx8mm.h | 49 ++++++++++
4 files changed, 168 insertions(+)
create mode 100644 drivers/interconnect/imx/imx8mm.c
create mode 100644 include/dt-bindings/interconnect/imx8mm.h
diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index 45fbae7007af..2f06cb1f81c3 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -1,5 +1,9 @@
config INTERCONNECT_IMX
bool "i.MX interconnect drivers"
depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
help
Generic interconnect driver for i.MX SOCs
+
+config INTERCONNECT_IMX8MM
+ def_bool y
+ depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index bb92fd9fe4a5..5f658c1608a6 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
+obj-$(CONFIG_INTERCONNECT_IMX8MM) += imx8mm.o
diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
new file mode 100644
index 000000000000..5bed7babff96
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mm.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/interconnect/imx8mm.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
+ .devfreq_name = "dram",
+ .bw_mul = 1,
+ .bw_div = 16,
+};
+
+static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
+ .devfreq_name = "noc",
+ .bw_mul = 1,
+ .bw_div = 16,
+};
+
+/*
+ * Describe bus masters, slaves and connections between them
+ *
+ * This is a simplified subset of the bus diagram, there are several other
+ * PL301 nics which are skipped/merged into PL301_MAIN
+ */
+static struct imx_icc_node_desc nodes[] = {
+ DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
+ 2, IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
+
+ DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
+ DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
+ DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
+
+ /* VPUMIX */
+ DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
+ DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
+ DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
+ DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, 1, IMX8MM_ICN_NOC),
+
+ /* GPUMIX */
+ DEFINE_BUS_MASTER("GPU 2D", IMX8MM_ICM_GPU2D, IMX8MM_ICN_GPU),
+ DEFINE_BUS_MASTER("GPU 3D", IMX8MM_ICM_GPU3D, IMX8MM_ICN_GPU),
+ DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, NULL, 1, IMX8MM_ICN_NOC),
+
+ /* DISPLAYMIX */
+ DEFINE_BUS_MASTER("CSI", IMX8MM_ICM_CSI, IMX8MM_ICN_MIPI),
+ DEFINE_BUS_MASTER("LCDIF", IMX8MM_ICM_LCDIF, IMX8MM_ICN_MIPI),
+ DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, NULL, 1, IMX8MM_ICN_NOC),
+
+ /* HSIO */
+ DEFINE_BUS_MASTER("USB1", IMX8MM_ICM_USB1, IMX8MM_ICN_HSIO),
+ DEFINE_BUS_MASTER("USB2", IMX8MM_ICM_USB2, IMX8MM_ICN_HSIO),
+ DEFINE_BUS_MASTER("PCIE", IMX8MM_ICM_PCIE, IMX8MM_ICN_HSIO),
+ DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, NULL, 1, IMX8MM_ICN_NOC),
+
+ /* Audio */
+ DEFINE_BUS_MASTER("SDMA2", IMX8MM_ICM_SDMA2, IMX8MM_ICN_AUDIO),
+ DEFINE_BUS_MASTER("SDMA3", IMX8MM_ICM_SDMA3, IMX8MM_ICN_AUDIO),
+ DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, NULL, 1, IMX8MM_ICN_MAIN),
+
+ /* Ethernet */
+ DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
+ DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, 1, IMX8MM_ICN_MAIN),
+
+ /* Other */
+ DEFINE_BUS_MASTER("SDMA1", IMX8MM_ICM_SDMA1, IMX8MM_ICN_MAIN),
+ DEFINE_BUS_MASTER("NAND", IMX8MM_ICM_NAND, IMX8MM_ICN_MAIN),
+ DEFINE_BUS_MASTER("USDHC1", IMX8MM_ICM_USDHC1, IMX8MM_ICN_MAIN),
+ DEFINE_BUS_MASTER("USDHC2", IMX8MM_ICM_USDHC2, IMX8MM_ICN_MAIN),
+ DEFINE_BUS_MASTER("USDHC3", IMX8MM_ICM_USDHC3, IMX8MM_ICN_MAIN),
+ DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN, NULL,
+ 2, IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
+};
+
+static int imx8mm_icc_probe(struct platform_device *pdev)
+{
+ return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+}
+
+static int imx8mm_icc_remove(struct platform_device *pdev)
+{
+ return imx_icc_unregister(pdev);
+}
+
+static const struct of_device_id imx_icc_of_match[] = {
+ { .compatible = "fsl,imx8mm-interconnect" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, imx_icc_of_match);
+
+static struct platform_driver imx8mm_icc_driver = {
+ .probe = imx8mm_icc_probe,
+ .remove = imx8mm_icc_remove,
+ .driver = {
+ .name = "imx8mm-interconnect",
+ .of_match_table = imx_icc_of_match,
+ },
+};
+
+builtin_platform_driver(imx8mm_icc_driver);
+MODULE_AUTHOR("Alexandre Bailon <abailon@baylibre.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/interconnect/imx8mm.h b/include/dt-bindings/interconnect/imx8mm.h
new file mode 100644
index 000000000000..5404f2af15c3
--- /dev/null
+++ b/include/dt-bindings/interconnect/imx8mm.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ */
+
+#ifndef __IMX8MM_ICM_INTERCONNECT_IDS_H
+#define __IMX8MM_ICM_INTERCONNECT_IDS_H
+
+#define IMX8MM_ICN_NOC 1
+#define IMX8MM_ICS_DRAM 2
+#define IMX8MM_ICS_OCRAM 3
+#define IMX8MM_ICM_A53 4
+
+#define IMX8MM_ICM_VPU_H1 5
+#define IMX8MM_ICM_VPU_G1 6
+#define IMX8MM_ICM_VPU_G2 7
+#define IMX8MM_ICN_VIDEO 8
+
+#define IMX8MM_ICM_GPU2D 9
+#define IMX8MM_ICM_GPU3D 10
+#define IMX8MM_ICN_GPU 11
+
+#define IMX8MM_ICM_CSI 12
+#define IMX8MM_ICM_LCDIF 13
+#define IMX8MM_ICN_MIPI 14
+
+#define IMX8MM_ICM_USB1 15
+#define IMX8MM_ICM_USB2 16
+#define IMX8MM_ICM_PCIE 17
+#define IMX8MM_ICN_HSIO 18
+
+#define IMX8MM_ICM_SDMA2 19
+#define IMX8MM_ICM_SDMA3 20
+#define IMX8MM_ICN_AUDIO 21
+
+#define IMX8MM_ICN_ENET 22
+#define IMX8MM_ICM_ENET 23
+
+#define IMX8MM_ICN_MAIN 24
+#define IMX8MM_ICM_NAND 25
+#define IMX8MM_ICM_SDMA1 26
+#define IMX8MM_ICM_USDHC1 27
+#define IMX8MM_ICM_USDHC2 28
+#define IMX8MM_ICM_USDHC3 29
+
+#endif /* __IMX8MM_ICM_INTERCONNECT_IDS_H */
--
2.17.1
^ permalink raw reply related
* Re: [alsa-devel] [PATCH v2 1/7] ASoC: fsl_sai: Add registers definition for multiple datalines
From: Daniel Baluta @ 2019-08-06 11:15 UTC (permalink / raw)
To: Nicolin Chen
Cc: Mark Brown, Daniel Baluta, Devicetree List, Linux-ALSA,
Pengutronix Kernel Team, Timur Tabi, Rob Herring, S.j. Wang,
Angus Ainslie (Purism), Takashi Iwai, Linux Kernel Mailing List,
dl-linux-imx, Viorel Suman, Fabio Estevam, Mihai Serban,
Lucas Stach
In-Reply-To: <20190730075934.GA5892@Asurada>
On Tue, Jul 30, 2019 at 10:59 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Mon, Jul 29, 2019 at 09:20:01PM +0100, Mark Brown wrote:
> > On Mon, Jul 29, 2019 at 10:57:43PM +0300, Daniel Baluta wrote:
> > > On Mon, Jul 29, 2019 at 10:42 PM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > > On Sun, Jul 28, 2019 at 10:24:23PM +0300, Daniel Baluta wrote:
> >
> > > > > @@ -704,7 +711,14 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
> > > > > case FSL_SAI_TCR3:
> > > > > case FSL_SAI_TCR4:
> > > > > case FSL_SAI_TCR5:
> > > > > - case FSL_SAI_TFR:
> > > > > + case FSL_SAI_TFR0:
> >
> > > > A tricky thing here is that those SAI instances on older SoC don't
> > > > support multi data lines physically, while seemly having registers
> > > > pre-defined. So your change doesn't sound doing anything wrong to
> > > > them at all, I am still wondering if it is necessary to apply them
> > > > to newer compatible only though, as for older compatibles of SAI,
> > > > these registers would be useless and confusing if being exposed.
> >
> > > > What do you think?
> >
> > > Yes, I thought about this too. But, I tried to keep the code as short
> > > as possible and technically it is not wrong. When 1 data line is supported
> > > for example application will only care about TDR0, TFR0, etc.
> >
> > So long as it's safe to read the registers (you don't get a bus error or
> > anything) I'd say it's more trouble than it's worth to have separate
> > regmap configuations just for this. The main reasons for restricting
> > readability are where there's physical problems with doing the reads or
> > to keep the size of the debugfs files under control for usability and
> > performance reasons.
>
> Thanks for the input, Mark.
>
> Daniel, did you get a chance to test it on older SoCs? At least
> nothing breaks like bus errors?
Tested on imx6sx-sdb, everything looks good. No bus errors.
^ permalink raw reply
* RE: [PATCH RFC 3/7] pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
From: Yoshihiro Shimoda @ 2019-08-06 11:38 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland,
GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>, Linux-Renesas
In-Reply-To: <CAMuHMdUAVGbvn0D=UkqhY6RpO70MR-4GBC8i931a+fV9f6+njg@mail.gmail.com>
Hi Geert-san,
Thank you for your review!
> From: Geert Uytterhoeven, Sent: Tuesday, August 6, 2019 6:03 PM
>
> Hi Shimoda-san,
>
> On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > R-Car PWM controller requires the gpio to output zero duty,
> > this patch allows to roll it back from gpio to mux when the gpio
> > is freed.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/sh-pfc/pinctrl.c
> > +++ b/drivers/pinctrl/sh-pfc/pinctrl.c
> > @@ -26,6 +26,7 @@
> > #include "../pinconf.h"
> >
> > struct sh_pfc_pin_config {
> > + unsigned int mux_mark;
>
> Due to padding, adding this field will increase memory consumption by
> 6 bytes per pin.
I see.
> Probably sh_pfc_pin_group.{pins,mux} should be changed from unsigned int
> to u16, but that's out of scope for this patch.
I got it.
> > bool mux_set;
> > bool gpio_enabled;
> > };
> > @@ -353,6 +354,15 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> > spin_lock_irqsave(&pfc->lock, flags);
> >
> > for (i = 0; i < grp->nr_pins; ++i) {
> > + int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]);
> > + struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
> > +
> > + /*
> > + * This doesn't assume the order which gpios are enabled
> > + * and then mux is set.
>
> I'm sorry, I don't understand what you mean?
> Can you please reword or elaborate?
I was also difficult to remember what I meant...
Anyway, this meant,
1) if a device has the default pinctrl-0 property, the set_mux() ops is called
before the device driver's probe() function is called by pinctrl_bind_pins() first,
2) so that any device drivers cannot call gpiod_get() before the 1).
However, this comments don't cover an imbalance pinctrl/gpio handling.
For example (as pseudo):
- SCIF driver uses SCIF2 pinctrl,
- but, IOMMU driver gets the SCIF2 pins before SCIF driver is probed.
So, I'd like to revise the comments as following. What do you think?
--
This driver cannot manage both gpio and mux when the gpio pin
is already enabled. So, this function failed.
--
> > + */
> > + WARN_ON(cfg->gpio_enabled);
>
> Can this actually happen?
This cannot happen actually.
> Should this cause a failure instead?
I think so.
> > +
> > ret = sh_pfc_config_mux(pfc, grp->mux[i], PINMUX_TYPE_FUNCTION);
> > if (ret < 0)
> > goto done;
> > @@ -364,6 +374,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> > struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
> >
> > cfg->mux_set = true;
> > + cfg->mux_mark = grp->mux[i];
> > }
> >
> > done:
> > @@ -417,6 +428,9 @@ static void sh_pfc_gpio_disable_free(struct pinctrl_dev *pctldev,
> >
> > spin_lock_irqsave(&pfc->lock, flags);
> > cfg->gpio_enabled = false;
> > + /* If mux is already set, this configure it here */
>
> configures
Oops! I'll fix it.
> > + if (cfg->mux_set)
> > + sh_pfc_config_mux(pfc, cfg->mux_mark, PINMUX_TYPE_FUNCTION);
>
> Have you considered the case where more than one pin of a pinmux group
> was used as a GPIO? In that case sh_pfc_gpio_disable_free() will be called
> multiple times, possibly with the same mux_mark.
I haven't considered the case. But, about the mux_mark, I checked the values and then
they are not the same.
For example (debug printk patch):
diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
index bc29066..fdac71b 100644
--- a/drivers/pinctrl/sh-pfc/pinctrl.c
+++ b/drivers/pinctrl/sh-pfc/pinctrl.c
@@ -349,7 +349,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
unsigned int i;
int ret = 0;
- dev_dbg(pctldev->dev, "Configuring pin group %s\n", grp->name);
+ dev_info(pctldev->dev, "Configuring pin group %s\n", grp->name);
spin_lock_irqsave(&pfc->lock, flags);
@@ -375,6 +375,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
cfg->mux_set = true;
cfg->mux_mark = grp->mux[i];
+ dev_info(pctldev->dev, "%d: %x\n", i, cfg->mux_mark);
}
done:
--
2.7.4
For example (log):
[ 0.497647] sh-pfc e6060000.pin-controller: Configuring pin group scif2_data_a
[ 0.497711] sh-pfc e6060000.pin-controller: 0: 77b
[ 0.497715] sh-pfc e6060000.pin-controller: 1: 760
Best regards,
Yoshihiro Shimoda
> I don't think this will cause issues, though.
>
> > spin_unlock_irqrestore(&pfc->lock, flags);
> > }
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply related
* RE: [PATCH RFC 5/7] pwm: rcar: remove a redundant condition in rcar_pwm_apply()
From: Yoshihiro Shimoda @ 2019-08-06 11:39 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland,
GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>, Linux-Renesas
In-Reply-To: <CAMuHMdWw1Gh_CxgiO5gd+MY0vUvWX_ACDj+L3_Wcomkaf5Oo4Q@mail.gmail.com>
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Tuesday, August 6, 2019 6:06 PM
>
> Hi Shimoda-san,
>
> On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Since the rcar_pwm_apply() has already check whehter state->enabled
> > is not set or not, this patch removes a redundant condition.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Thank you for your review!
> This is completely independent from the rest of the series, and can be applied
> immediately, right?
That's right.
Best regards,
Yoshihiro Shimoda
> Gr{oetje,eeting}s,
>
> Geert
>
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v4 10/10] rtc: Add support for the MediaTek MT6358 RTC
From: Ran Bi @ 2019-08-06 11:41 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Hsin-Hsiung Wang, Lee Jones, Rob Herring, Matthias Brugger,
Mark Brown, Mark Rutland, Liam Girdwood, Eddie Huang, Sean Wang,
Alessandro Zummo, Thomas Gleixner, Richard Fontana, Kate Stewart,
Allison Randal, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, linux-rtc, srv_heupstream
In-Reply-To: <20190805072338.GB3600@piout.net>
Hi Belloni,
On Mon, 2019-08-05 at 09:23 +0200, Alexandre Belloni wrote:
> Hi,
>
> The subject should be:
>
> "rtc: mt6397: Add support for the MediaTek MT6358 RTC"
Will be changed at next patch.
> > +struct mtk_rtc_compatible {
>
> I would name that struct mtk_rtc_data
>
> > + u32 wrtgr_addr;
>
> and this member should be wrtgr_offset or simply wrtgr.
>
Will be changed at next patch.
> >
> > + of_id = of_match_device(mt6397_rtc_of_match, &pdev->dev);
> > + if (!of_id) {
> > + dev_err(&pdev->dev, "Failed to probe of_node\n");
> > + return -EINVAL;
>
> This will never happen because probe would not be called if there is no
> match. You could also use of_device_get_match_data to avoid having to
> move the of_device_id table.
>
Will use of_device_get_match_data() function instead of
of_match_device() function.
^ permalink raw reply
* Re: [PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver
From: Ardelean, Alexandru @ 2019-08-06 11:47 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805141100.GG24275@lunn.ch>
On Mon, 2019-08-05 at 16:11 +0200, Andrew Lunn wrote:
> [External]
>
> > + adi,rx-internal-delay:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: |
> > + RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> > + default value is 0 (which represents 2 ns)
> > + enum: [ 0, 1, 2, 6, 7 ]
>
> We want these numbers to be in ns. So the default value would actually
> be 2. The driver needs to convert the number in DT to a value to poke
> into a PHY register. Please rename the property adi,rx-internal-delay-ns.
>
I just realized: this will probably have to be pico-seconds.
Some delays are 1.60 ns, which are not easy to represent in in ns in DT.
The values here are actually the register values corresponding to the delays.
> > +
> > + adi,tx-internal-delay:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: |
> > + RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> > + default value is 0 (which represents 2 ns)
> > + enum: [ 0, 1, 2, 6, 7 ]
>
> Same here.
>
> > +
> > + adi,fifo-depth:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: |
> > + When operating in RMII mode, this option configures the FIFO depth.
> > + See `dt-bindings/net/adin.h`.
> > + enum: [ 0, 1, 2, 3, 4, 5 ]
>
> Units? You should probably rename this adi,fifo-depth-bits and list
> the valid values in bits.
>
> > +
> > + adi,eee-enabled:
> > + description: |
> > + Advertise EEE capabilities on power-up/init (default disabled)
> > + type: boolean
>
> It is not the PHY which decides this. The MAC indicates if it is EEE
> capable to phylib. phylib looks into the PHY registers to determine if
> the PHY supports EEE. phylib will then enable EEE
> advertisement. Please remove this, and ensure EEE is disabled by
> default.
>
> Andrew
^ permalink raw reply
* RE: [PATCH RFC 2/7] pinctrl: sh-pfc: remove incomplete flag "cfg->type"
From: Yoshihiro Shimoda @ 2019-08-06 11:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland,
GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>, Linux-Renesas
In-Reply-To: <CAMuHMdVcAw_ApKMmrV7DaoJBGUZ1GzW3kmxnsTn72FtCGWhXPA@mail.gmail.com>
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Tuesday, August 6, 2019 6:24 PM
>
> Hi Shimoda-san,
>
> On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> > support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> > sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> > Now if we fix the cfg->type condition, it gets worse because:
> > - Some drivers might be deferred so that .set_mux() will be called
> > multiple times.
> > - In such the case, the sh-pfc driver returns -EBUSY even if
> > the group is the same, and then that driver fails to probe.
> >
> > Since the pinctrl subsystem already has such conditions according
> > to @set_mux and @gpio_request_enable, this patch just remove
> > the incomplete flag from sh-pfc/pinctrl.c.
>
> Do we need to set sh_pfc_pinmux_ops.strict = true?
If the .strict = true, the final pwm patch on this series failed with the following error:
[ 11.453716] sh-pfc e6060000.pin-controller: pin GP_2_7 already requested by e6e31000.pwm; cannot claim for e6052000.gpio:459
Best regards,
Yoshihiro Shimoda
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH RFC 3/7] pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
From: Geert Uytterhoeven @ 2019-08-06 12:01 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <OSBPR01MB4536870EEEE634B06199722ED8D50@OSBPR01MB4536.jpnprd01.prod.outlook.com>
Hi Shimoda-san,
On Tue, Aug 6, 2019 at 1:38 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, August 6, 2019 6:03 PM
> > On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > R-Car PWM controller requires the gpio to output zero duty,
> > > this patch allows to roll it back from gpio to mux when the gpio
> > > is freed.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/pinctrl/sh-pfc/pinctrl.c
> > > +++ b/drivers/pinctrl/sh-pfc/pinctrl.c
> > > @@ -26,6 +26,7 @@
> > > #include "../pinconf.h"
> > >
> > > struct sh_pfc_pin_config {
> > > + unsigned int mux_mark;
> >
> > Due to padding, adding this field will increase memory consumption by
> > 6 bytes per pin.
>
> I see.
>
> > Probably sh_pfc_pin_group.{pins,mux} should be changed from unsigned int
> > to u16, but that's out of scope for this patch.
>
> I got it.
For now, please don't worry about it. I can make that change later, as it will
affect all drivers.
> > > @@ -353,6 +354,15 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> > > spin_lock_irqsave(&pfc->lock, flags);
> > >
> > > for (i = 0; i < grp->nr_pins; ++i) {
> > > + int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]);
> > > + struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
> > > +
> > > + /*
> > > + * This doesn't assume the order which gpios are enabled
> > > + * and then mux is set.
> >
> > I'm sorry, I don't understand what you mean?
> > Can you please reword or elaborate?
>
> I was also difficult to remember what I meant...
> Anyway, this meant,
> 1) if a device has the default pinctrl-0 property, the set_mux() ops is called
> before the device driver's probe() function is called by pinctrl_bind_pins() first,
> 2) so that any device drivers cannot call gpiod_get() before the 1).
>
> However, this comments don't cover an imbalance pinctrl/gpio handling.
> For example (as pseudo):
> - SCIF driver uses SCIF2 pinctrl,
> - but, IOMMU driver gets the SCIF2 pins before SCIF driver is probed.
>
> So, I'd like to revise the comments as following. What do you think?
>
> --
> This driver cannot manage both gpio and mux when the gpio pin
> is already enabled. So, this function failed.
> --
>
> > > + */
> > > + WARN_ON(cfg->gpio_enabled);
> >
> > Can this actually happen?
>
> This cannot happen actually.
>
> > Should this cause a failure instead?
>
> I think so.
OK.
> > > + if (cfg->mux_set)
> > > + sh_pfc_config_mux(pfc, cfg->mux_mark, PINMUX_TYPE_FUNCTION);
> >
> > Have you considered the case where more than one pin of a pinmux group
> > was used as a GPIO? In that case sh_pfc_gpio_disable_free() will be called
> > multiple times, possibly with the same mux_mark.
>
> I haven't considered the case. But, about the mux_mark, I checked the values and then
> they are not the same.
IC. At first I thought they were the internal enum for the whole pin group, but
I was wrong.
They are the mux *_MARK enu, which is unique for each pin/function combo.
> For example (debug printk patch):
> diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
> index bc29066..fdac71b 100644
> --- a/drivers/pinctrl/sh-pfc/pinctrl.c
> +++ b/drivers/pinctrl/sh-pfc/pinctrl.c
> @@ -349,7 +349,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> unsigned int i;
> int ret = 0;
>
> - dev_dbg(pctldev->dev, "Configuring pin group %s\n", grp->name);
> + dev_info(pctldev->dev, "Configuring pin group %s\n", grp->name);
>
> spin_lock_irqsave(&pfc->lock, flags);
>
> @@ -375,6 +375,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
>
> cfg->mux_set = true;
> cfg->mux_mark = grp->mux[i];
> + dev_info(pctldev->dev, "%d: %x\n", i, cfg->mux_mark);
> }
>
> done:
> --
> 2.7.4
>
> For example (log):
> [ 0.497647] sh-pfc e6060000.pin-controller: Configuring pin group scif2_data_a
> [ 0.497711] sh-pfc e6060000.pin-controller: 0: 77b
> [ 0.497715] sh-pfc e6060000.pin-controller: 1: 760
Thanks for checking!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v2 0/3] drm/meson: convert bindings to YAML schemas
From: Neil Armstrong @ 2019-08-06 12:44 UTC (permalink / raw)
To: robh+dt; +Cc: Neil Armstrong, devicetree, dri-devel, linux-amlogic,
linux-kernel
This patchset converts the existing text bindings to YAML schemas.
Those bindings have a lot of texts, thus is interesting to convert.
All have been tested using :
$ make ARCH=arm64 dtbs_check
Issues with the amlogic arm64 DTs has already been identified thanks
to the validation scripts. The DT fixes will be pushed once these yaml
bindings are acked.
Neil Armstrong (3):
dt-bindings: display: amlogic,meson-dw-hdmi: convert to yaml
dt-bindings: display: amlogic,meson-vpu: convert to yaml
MAINTAINERS: Update with Amlogic DRM bindings converted as YAML
.../display/amlogic,meson-dw-hdmi.txt | 119 --------------
.../display/amlogic,meson-dw-hdmi.yaml | 150 ++++++++++++++++++
.../bindings/display/amlogic,meson-vpu.txt | 121 --------------
.../bindings/display/amlogic,meson-vpu.yaml | 138 ++++++++++++++++
MAINTAINERS | 4 +-
5 files changed, 290 insertions(+), 242 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.txt
create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.yaml
delete mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml
--
2.22.0
^ permalink raw reply
* [PATCH v2 1/3] dt-bindings: display: amlogic,meson-dw-hdmi: convert to yaml
From: Neil Armstrong @ 2019-08-06 12:44 UTC (permalink / raw)
To: robh+dt; +Cc: Neil Armstrong, devicetree, dri-devel, linux-amlogic,
linux-kernel
In-Reply-To: <20190806124416.15561-1-narmstrong@baylibre.com>
Now that we have the DT validation in place, let's convert the device tree
bindings for the Amlogic Synopsys DW-HDMI specifics over to YAML schemas.
The original example and usage of clock-names uses a reversed "isfr"
and "iahb" clock-names, the rewritten YAML bindings uses the reversed
instead of fixing the device trees order.
The #sound-dai-cells optional property has been added to match this node
as a sound dai.
The port connection table has been dropped in favor of a description
of each port.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../display/amlogic,meson-dw-hdmi.txt | 119 --------------
.../display/amlogic,meson-dw-hdmi.yaml | 150 ++++++++++++++++++
2 files changed, 150 insertions(+), 119 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.txt
create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.yaml
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.txt b/Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.txt
deleted file mode 100644
index 3a50a7862cf3..000000000000
--- a/Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.txt
+++ /dev/null
@@ -1,119 +0,0 @@
-Amlogic specific extensions to the Synopsys Designware HDMI Controller
-======================================================================
-
-The Amlogic Meson Synopsys Designware Integration is composed of :
-- A Synopsys DesignWare HDMI Controller IP
-- A TOP control block controlling the Clocks and PHY
-- A custom HDMI PHY in order to convert video to TMDS signal
- ___________________________________
-| HDMI TOP |<= HPD
-|___________________________________|
-| | |
-| Synopsys HDMI | HDMI PHY |=> TMDS
-| Controller |________________|
-|___________________________________|<=> DDC
-
-The HDMI TOP block only supports HPD sensing.
-The Synopsys HDMI Controller interrupt is routed through the
-TOP Block interrupt.
-Communication to the TOP Block and the Synopsys HDMI Controller is done
-via a pair of dedicated addr+read/write registers.
-The HDMI PHY is configured by registers in the HHI register block.
-
-Pixel data arrives in 4:4:4 format from the VENC block and the VPU HDMI mux
-selects either the ENCI encoder for the 576i or 480i formats or the ENCP
-encoder for all the other formats including interlaced HD formats.
-
-The VENC uses a DVI encoder on top of the ENCI or ENCP encoders to generate
-DVI timings for the HDMI controller.
-
-Amlogic Meson GXBB, GXL and GXM SoCs families embeds the Synopsys DesignWare
-HDMI TX IP version 2.01a with HDCP and I2C & S/PDIF
-audio source interfaces.
-
-Required properties:
-- compatible: value should be different for each SoC family as :
- - GXBB (S905) : "amlogic,meson-gxbb-dw-hdmi"
- - GXL (S905X, S905D) : "amlogic,meson-gxl-dw-hdmi"
- - GXM (S912) : "amlogic,meson-gxm-dw-hdmi"
- followed by the common "amlogic,meson-gx-dw-hdmi"
- - G12A (S905X2, S905Y2, S905D2) : "amlogic,meson-g12a-dw-hdmi"
-- reg: Physical base address and length of the controller's registers.
-- interrupts: The HDMI interrupt number
-- clocks, clock-names : must have the phandles to the HDMI iahb and isfr clocks,
- and the Amlogic Meson venci clocks as described in
- Documentation/devicetree/bindings/clock/clock-bindings.txt,
- the clocks are soc specific, the clock-names should be "iahb", "isfr", "venci"
-- resets, resets-names: must have the phandles to the HDMI apb, glue and phy
- resets as described in :
- Documentation/devicetree/bindings/reset/reset.txt,
- the reset-names should be "hdmitx_apb", "hdmitx", "hdmitx_phy"
-
-Optional properties:
-- hdmi-supply: Optional phandle to an external 5V regulator to power the HDMI
- logic, as described in the file ../regulator/regulator.txt
-
-Required nodes:
-
-The connections to the HDMI ports are modeled using the OF graph
-bindings specified in Documentation/devicetree/bindings/graph.txt.
-
-The following table lists for each supported model the port number
-corresponding to each HDMI output and input.
-
- Port 0 Port 1
------------------------------------------
- S905 (GXBB) VENC Input TMDS Output
- S905X (GXL) VENC Input TMDS Output
- S905D (GXL) VENC Input TMDS Output
- S912 (GXM) VENC Input TMDS Output
- S905X2 (G12A) VENC Input TMDS Output
- S905Y2 (G12A) VENC Input TMDS Output
- S905D2 (G12A) VENC Input TMDS Output
-
-Example:
-
-hdmi-connector {
- compatible = "hdmi-connector";
- type = "a";
-
- port {
- hdmi_connector_in: endpoint {
- remote-endpoint = <&hdmi_tx_tmds_out>;
- };
- };
-};
-
-hdmi_tx: hdmi-tx@c883a000 {
- compatible = "amlogic,meson-gxbb-dw-hdmi", "amlogic,meson-gx-dw-hdmi";
- reg = <0x0 0xc883a000 0x0 0x1c>;
- interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
- resets = <&reset RESET_HDMITX_CAPB3>,
- <&reset RESET_HDMI_SYSTEM_RESET>,
- <&reset RESET_HDMI_TX>;
- reset-names = "hdmitx_apb", "hdmitx", "hdmitx_phy";
- clocks = <&clkc CLKID_HDMI_PCLK>,
- <&clkc CLKID_CLK81>,
- <&clkc CLKID_GCLK_VENCI_INT0>;
- clock-names = "isfr", "iahb", "venci";
- #address-cells = <1>;
- #size-cells = <0>;
-
- /* VPU VENC Input */
- hdmi_tx_venc_port: port@0 {
- reg = <0>;
-
- hdmi_tx_in: endpoint {
- remote-endpoint = <&hdmi_tx_out>;
- };
- };
-
- /* TMDS Output */
- hdmi_tx_tmds_port: port@1 {
- reg = <1>;
-
- hdmi_tx_tmds_out: endpoint {
- remote-endpoint = <&hdmi_connector_in>;
- };
- };
-};
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.yaml b/Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.yaml
new file mode 100644
index 000000000000..fb747682006d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.yaml
@@ -0,0 +1,150 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 BayLibre, SAS
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/display/amlogic,meson-dw-hdmi.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic specific extensions to the Synopsys Designware HDMI Controller
+
+maintainers:
+ - Neil Armstrong <narmstrong@baylibre.com>
+
+description: |
+ The Amlogic Meson Synopsys Designware Integration is composed of
+ - A Synopsys DesignWare HDMI Controller IP
+ - A TOP control block controlling the Clocks and PHY
+ - A custom HDMI PHY in order to convert video to TMDS signal
+ ___________________________________
+ | HDMI TOP |<= HPD
+ |___________________________________|
+ | | |
+ | Synopsys HDMI | HDMI PHY |=> TMDS
+ | Controller |________________|
+ |___________________________________|<=> DDC
+
+ The HDMI TOP block only supports HPD sensing.
+ The Synopsys HDMI Controller interrupt is routed through the
+ TOP Block interrupt.
+ Communication to the TOP Block and the Synopsys HDMI Controller is done
+ via a pair of dedicated addr+read/write registers.
+ The HDMI PHY is configured by registers in the HHI register block.
+
+ Pixel data arrives in "4:4:4" format from the VENC block and the VPU HDMI mux
+ selects either the ENCI encoder for the 576i or 480i formats or the ENCP
+ encoder for all the other formats including interlaced HD formats.
+
+ The VENC uses a DVI encoder on top of the ENCI or ENCP encoders to generate
+ DVI timings for the HDMI controller.
+
+ Amlogic Meson GXBB, GXL and GXM SoCs families embeds the Synopsys DesignWare
+ HDMI TX IP version 2.01a with HDCP and I2C & S/PDIF
+ audio source interfaces.
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - amlogic,meson-gxbb-dw-hdmi # GXBB (S905)
+ - amlogic,meson-gxl-dw-hdmi # GXL (S905X, S905D)
+ - amlogic,meson-gxm-dw-hdmi # GXM (S912)
+ - const: amlogic,meson-gx-dw-hdmi
+ - enum:
+ - amlogic,meson-g12a-dw-hdmi # G12A (S905X2, S905Y2, S905D2)
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 3
+
+ clock-names:
+ items:
+ - const: isfr
+ - const: iahb
+ - const: venci
+
+ resets:
+ minItems: 3
+
+ reset-names:
+ items:
+ - const: hdmitx_apb
+ - const: hdmitx
+ - const: hdmitx_phy
+
+ hdmi-supply:
+ description: phandle to an external 5V regulator to power the HDMI logic
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/phandle
+
+ port@0:
+ type: object
+ description:
+ A port node pointing to the VENC Input port node.
+
+ port@1:
+ type: object
+ description:
+ A port node pointing to the TMDS Output port node.
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ "#sound-dai-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - port@0
+ - port@1
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ hdmi_tx: hdmi-tx@c883a000 {
+ compatible = "amlogic,meson-gxbb-dw-hdmi", "amlogic,meson-gx-dw-hdmi";
+ reg = <0xc883a000 0x1c>;
+ interrupts = <57>;
+ resets = <&reset_apb>, <&reset_hdmitx>, <&reset_hdmitx_phy>;
+ reset-names = "hdmitx_apb", "hdmitx", "hdmitx_phy";
+ clocks = <&clk_isfr>, <&clk_iahb>, <&clk_venci>;
+ clock-names = "isfr", "iahb", "venci";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* VPU VENC Input */
+ hdmi_tx_venc_port: port@0 {
+ reg = <0>;
+
+ hdmi_tx_in: endpoint {
+ remote-endpoint = <&hdmi_tx_out>;
+ };
+ };
+
+ /* TMDS Output */
+ hdmi_tx_tmds_port: port@1 {
+ reg = <1>;
+
+ hdmi_tx_tmds_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+ };
+ };
+
--
2.22.0
^ permalink raw reply related
* [PATCH v2 2/3] dt-bindings: display: amlogic,meson-vpu: convert to yaml
From: Neil Armstrong @ 2019-08-06 12:44 UTC (permalink / raw)
To: robh+dt; +Cc: Neil Armstrong, devicetree, dri-devel, linux-amlogic,
linux-kernel
In-Reply-To: <20190806124416.15561-1-narmstrong@baylibre.com>
Now that we have the DT validation in place, let's convert the device tree
bindings for the Amlogic Display Controller over to YAML schemas.
The original example has a leftover "dmc" memory cell, that has been
removed in the yaml rewrite.
The port connection table has been dropped in favor of a description
of each port.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../bindings/display/amlogic,meson-vpu.txt | 121 ---------------
.../bindings/display/amlogic,meson-vpu.yaml | 138 ++++++++++++++++++
2 files changed, 138 insertions(+), 121 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
deleted file mode 100644
index be40a780501c..000000000000
--- a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
+++ /dev/null
@@ -1,121 +0,0 @@
-Amlogic Meson Display Controller
-================================
-
-The Amlogic Meson Display controller is composed of several components
-that are going to be documented below:
-
-DMC|---------------VPU (Video Processing Unit)----------------|------HHI------|
- | vd1 _______ _____________ _________________ | |
-D |-------| |----| | | | | HDMI PLL |
-D | vd2 | VIU | | Video Post | | Video Encoders |<---|-----VCLK |
-R |-------| |----| Processing | | | | |
- | osd2 | | | |---| Enci ----------|----|-----VDAC------|
-R |-------| CSC |----| Scalers | | Encp ----------|----|----HDMI-TX----|
-A | osd1 | | | Blenders | | Encl ----------|----|---------------|
-M |-------|______|----|____________| |________________| | |
-___|__________________________________________________________|_______________|
-
-
-VIU: Video Input Unit
----------------------
-
-The Video Input Unit is in charge of the pixel scanout from the DDR memory.
-It fetches the frames addresses, stride and parameters from the "Canvas" memory.
-This part is also in charge of the CSC (Colorspace Conversion).
-It can handle 2 OSD Planes and 2 Video Planes.
-
-VPP: Video Post Processing
---------------------------
-
-The Video Post Processing is in charge of the scaling and blending of the
-various planes into a single pixel stream.
-There is a special "pre-blending" used by the video planes with a dedicated
-scaler and a "post-blending" to merge with the OSD Planes.
-The OSD planes also have a dedicated scaler for one of the OSD.
-
-VENC: Video Encoders
---------------------
-
-The VENC is composed of the multiple pixel encoders :
- - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
- - ENCP : Progressive Video Encoder for HDMI
- - ENCL : LCD LVDS Encoder
-The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
-tree and provides the scanout clock to the VPP and VIU.
-The ENCI is connected to a single VDAC for Composite Output.
-The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
-
-Device Tree Bindings:
----------------------
-
-VPU: Video Processing Unit
---------------------------
-
-Required properties:
-- compatible: value should be different for each SoC family as :
- - GXBB (S905) : "amlogic,meson-gxbb-vpu"
- - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
- - GXM (S912) : "amlogic,meson-gxm-vpu"
- followed by the common "amlogic,meson-gx-vpu"
- - G12A (S905X2, S905Y2, S905D2) : "amlogic,meson-g12a-vpu"
-- reg: base address and size of he following memory-mapped regions :
- - vpu
- - hhi
-- reg-names: should contain the names of the previous memory regions
-- interrupts: should contain the VENC Vsync interrupt number
-- amlogic,canvas: phandle to canvas provider node as described in the file
- ../soc/amlogic/amlogic,canvas.txt
-
-Optional properties:
-- power-domains: Optional phandle to associated power domain as described in
- the file ../power/power_domain.txt
-
-Required nodes:
-
-The connections to the VPU output video ports are modeled using the OF graph
-bindings specified in Documentation/devicetree/bindings/graph.txt.
-
-The following table lists for each supported model the port number
-corresponding to each VPU output.
-
- Port 0 Port 1
------------------------------------------
- S905 (GXBB) CVBS VDAC HDMI-TX
- S905X (GXL) CVBS VDAC HDMI-TX
- S905D (GXL) CVBS VDAC HDMI-TX
- S912 (GXM) CVBS VDAC HDMI-TX
- S905X2 (G12A) CVBS VDAC HDMI-TX
- S905Y2 (G12A) CVBS VDAC HDMI-TX
- S905D2 (G12A) CVBS VDAC HDMI-TX
-
-Example:
-
-tv-connector {
- compatible = "composite-video-connector";
-
- port {
- tv_connector_in: endpoint {
- remote-endpoint = <&cvbs_vdac_out>;
- };
- };
-};
-
-vpu: vpu@d0100000 {
- compatible = "amlogic,meson-gxbb-vpu";
- reg = <0x0 0xd0100000 0x0 0x100000>,
- <0x0 0xc883c000 0x0 0x1000>,
- <0x0 0xc8838000 0x0 0x1000>;
- reg-names = "vpu", "hhi", "dmc";
- interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- /* CVBS VDAC output port */
- port@0 {
- reg = <0>;
-
- cvbs_vdac_out: endpoint {
- remote-endpoint = <&tv_connector_in>;
- };
- };
-};
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml
new file mode 100644
index 000000000000..71f572f6cb3b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml
@@ -0,0 +1,138 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 BayLibre, SAS
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/display/amlogic,meson-vpu.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson Display Controller
+
+maintainers:
+ - Neil Armstrong <narmstrong@baylibre.com>
+
+description: |
+ The Amlogic Meson Display controller is composed of several components
+ that are going to be documented below
+
+ DMC|---------------VPU (Video Processing Unit)----------------|------HHI------|
+ | vd1 _______ _____________ _________________ | |
+ D |-------| |----| | | | | HDMI PLL |
+ D | vd2 | VIU | | Video Post | | Video Encoders |<---|-----VCLK |
+ R |-------| |----| Processing | | | | |
+ | osd2 | | | |---| Enci ----------|----|-----VDAC------|
+ R |-------| CSC |----| Scalers | | Encp ----------|----|----HDMI-TX----|
+ A | osd1 | | | Blenders | | Encl ----------|----|---------------|
+ M |-------|______|----|____________| |________________| | |
+ ___|__________________________________________________________|_______________|
+
+
+ VIU: Video Input Unit
+ ---------------------
+
+ The Video Input Unit is in charge of the pixel scanout from the DDR memory.
+ It fetches the frames addresses, stride and parameters from the "Canvas" memory.
+ This part is also in charge of the CSC (Colorspace Conversion).
+ It can handle 2 OSD Planes and 2 Video Planes.
+
+ VPP: Video Post Processing
+ --------------------------
+
+ The Video Post Processing is in charge of the scaling and blending of the
+ various planes into a single pixel stream.
+ There is a special "pre-blending" used by the video planes with a dedicated
+ scaler and a "post-blending" to merge with the OSD Planes.
+ The OSD planes also have a dedicated scaler for one of the OSD.
+
+ VENC: Video Encoders
+ --------------------
+
+ The VENC is composed of the multiple pixel encoders
+ - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
+ - ENCP : Progressive Video Encoder for HDMI
+ - ENCL : LCD LVDS Encoder
+ The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
+ tree and provides the scanout clock to the VPP and VIU.
+ The ENCI is connected to a single VDAC for Composite Output.
+ The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - amlogic,meson-gxbb-vpu # GXBB (S905)
+ - amlogic,meson-gxl-vpu # GXL (S905X, S905D)
+ - amlogic,meson-gxm-vpu # GXM (S912)
+ - const: amlogic,meson-gx-vpu
+ - enum:
+ - amlogic,meson-g12a-vpu # G12A (S905X2, S905Y2, S905D2)
+
+ reg:
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: vpu
+ - const: hhi
+
+ interrupts:
+ maxItems: 1
+
+ power-domains:
+ description: phandle to the associated power domain
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/phandle
+
+ port@0:
+ type: object
+ description:
+ A port node pointing to the CVBS VDAC port node.
+
+ port@1:
+ type: object
+ description:
+ A port node pointing to the HDMI-TX port node.
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - port@0
+ - port@1
+ - "#address-cells"
+ - "#size-cells"
+
+examples:
+ - |
+ vpu: vpu@d0100000 {
+ compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
+ reg = <0xd0100000 0x100000>, <0xc883c000 0x1000>;
+ reg-names = "vpu", "hhi";
+ interrupts = <3>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* CVBS VDAC output port */
+ port@0 {
+ reg = <0>;
+
+ cvbs_vdac_out: endpoint {
+ remote-endpoint = <&tv_connector_in>;
+ };
+ };
+
+ /* HDMI TX output port */
+ port@1 {
+ reg = <1>;
+
+ hdmi_tx_out: endpoint {
+ remote-endpoint = <&hdmi_tx_in>;
+ };
+ };
+ };
--
2.22.0
^ permalink raw reply related
* Re: [PATCH] dt-bindings: net: meson-dwmac: convert to yaml
From: Neil Armstrong @ 2019-08-06 12:46 UTC (permalink / raw)
To: Rob Herring
Cc: Martin Blumenstingl, devicetree, netdev, linux-amlogic,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+efvvb1UK-Nas0G5XefLWwN7ebnqoevi+W=jj4r3E2dg@mail.gmail.com>
On 06/08/2019 00:09, Rob Herring wrote:
> On Mon, Aug 5, 2019 at 6:26 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> Now that we have the DT validation in place, let's convert the device tree
>> bindings for the Synopsys DWMAC Glue for Amlogic SoCs over to a YAML schemas.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> Rob,
>>
>> I keep getting :
>> .../devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: reg: [[3376480256, 65536], [3364046144, 8]] is too long
>
> Because snps,dwmac.yaml has:
>
> reg:
> maxItems: 1
>
> The schemas are applied separately and all have to be valid. You'll
> need to change snps,dwmac.yaml to:
>
> reg:
> minItems: 1
> maxItems: 2
>
>
> The schema error messages leave something to be desired. I wish the
> error messages said which schema is throwing the error.
Indeed, it fixed it
>
>> for the example DT
>>
>> and for the board DT :
>> ../amlogic/meson-gxl-s905x-libretech-cc.dt.yaml: ethernet@c9410000: reg: [[0, 3376480256, 0, 65536, 0, 3364046144, 0, 4]] is too short
>> ../amlogic/meson-gxl-s905x-nexbox-a95x.dt.yaml: soc: ethernet@c9410000:reg:0: [0, 3376480256, 0, 65536, 0, 3364046144, 0, 4] is too long
>>
>> and I don't know how to get rid of it.
>
> The first issue is the same as the above. The 2nd issue is the use of
> <> in dts files becomes stricter with the schema. Each entry in an
> array needs to be bracketed:
>
> reg = <0x0 0xc9410000 0x0 0x10000>,
> <0x0 0xc8834540 0x0 0x4>;
I did it but somehow it was overrided (with the same content) in another .dtsi
included file...
Sorry for the noise !
Neil
>
> Rob
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox