Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Exynos EHCI/OHCI: resolve conflict with the generic USB device bindings
From: Marek Szyprowski @ 2019-07-26  8:14 UTC (permalink / raw)
  To: linux-usb, linux-samsung-soc
  Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Markus Reichl, Måns Rullgård,
	Krzysztof Kozlowski, Peter Chen, Alan Stern, Rob Herring
In-Reply-To: <CGME20190726081504eucas1p1aea5376ff300f6baca21c5e1cb6e4b43@eucas1p1.samsung.com>

Dear All,

Commit 69bec7259853 ("USB: core: let USB device know device node") added
support for attaching devicetree node for USB devices. Those nodes are
children of their USB host controller. However Exynos EHCI and OHCI
driver bindings already define child-nodes for each physical root hub
port and assigns respective PHY controller and parameters to them. This
leads to the conflict. A workaround for it has been merged as commit
01d4071486fe ("usb: exynos: add workaround for the USB device bindings
conflict"), but it disabled support for USB device binding for Exynos
EHCI/OHCI controllers.

This patchset tries to resolve this binding conflict by changing Exynos
EHCI/OHCI bindings: PHYs are moved from the sub-nodes to a standard array
under the 'phys' property. Such solution has been suggested by Måns
Rullgård in the following thread: https://lkml.org/lkml/2019/5/13/228

To keep everything working during the transitional time, the changes has
been split into 2 steps. First the changes to Exynos OHCI and EHCI
drivers have to be merged, then in the next kernel release the DTS can be
finally updated to the new bindings.

This patchset has been tested on various Exynos boards with different
USB host controller configurations (Odroids family: X2, U3, XU3).

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:
v2:
- rearranged the code as suggested by Måns Rullgård, kept support for
  legacy bindings the Exynos EHCI/OHCI drivers

v1: https://patchwork.kernel.org/cover/10953495/
- initial version


Patch summary:

Marek Szyprowski (3):
  dt-bindings: switch Exynos EHCI/OHCI bindings to use array of generic
    PHYs
  usb: exynos: add support for getting PHYs from the standard dt array
  ARM: dts: exynos: Use standard arrays of generic PHYs for EHCI/OHCI
    devices

 .../devicetree/bindings/usb/exynos-usb.txt    | 41 +++++++------------
 arch/arm/boot/dts/exynos4.dtsi                | 28 ++-----------
 .../boot/dts/exynos4210-universal_c210.dts    |  8 +---
 arch/arm/boot/dts/exynos4412-itop-elite.dts   |  9 +---
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  8 +---
 arch/arm/boot/dts/exynos4412-odroidx.dts      |  5 +--
 arch/arm/boot/dts/exynos4412-origen.dts       |  9 +---
 arch/arm/boot/dts/exynos5250.dtsi             | 16 ++------
 arch/arm/boot/dts/exynos54xx.dtsi             | 18 ++------
 drivers/usb/host/ehci-exynos.c                | 23 +++++++++--
 drivers/usb/host/ohci-exynos.c                | 23 +++++++++--
 11 files changed, 74 insertions(+), 114 deletions(-)

-- 
2.17.1

^ permalink raw reply

* Re: [PATCH v2 00/14] Add support for QCOM Core Power Reduction
From: Viresh Kumar @ 2019-07-26  8:08 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: linux-pm, linux-arm-msm, jorge.ramirez-ortiz, sboyd, vireshk,
	bjorn.andersson, ulf.hansson, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190725104144.22924-1-niklas.cassel@linaro.org>

On 25-07-19, 12:41, Niklas Cassel wrote:
> This series adds support for Core Power Reduction (CPR), a form of
> Adaptive Voltage Scaling (AVS), found on certain Qualcomm SoCs.
> 
> This series is based on top of the qcs404 cpufreq patch series that
> hasn't landed yet:
> https://patchwork.kernel.org/project/linux-arm-msm/list/?series=137809
> 
> CPR is a technology that reduces core power on a CPU or on other device.
> It reads voltage settings from efuses (that have been written in
> production), it uses these voltage settings as initial values, for each
> OPP.
> 
> After moving to a certain OPP, CPR monitors dynamic factors such as
> temperature, etc. and adjusts the voltage for that frequency accordingly
> to save power and meet silicon characteristic requirements.
> 
> This driver has been developed together with Jorge Ramirez-Ortiz, and
> is based on an RFC by Stephen Boyd[1], which in turn is based on work
> by others on codeaurora.org[2].
> 
> [1] https://lkml.org/lkml/2015/9/18/833
> [2] https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/regulator/cpr-regulator.c?h=msm-3.10
> 
> Changes since V1:
> Added a new patch implementing dev_pm_opp_find_level_exact() in order to
> make the CPR OPP table in device tree cleaner.
> For more detailed changes, check the "Changes since V1" as comments in
> the individual patches, where applicable.

Applied patches [1-9/14] to cpufreq and OPP trees and done some
reordering as well to keep all binding patches together.

Rob's Ack is missing on two of the binding patches and I will add them
later once he provides it.

Everything should be available here for you to base rest of the stuff.

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git cpufreq/arm/linux-next

-- 
viresh

^ permalink raw reply

* Re: [RFC,v3 8/9] media: platform: Add Mediatek ISP P1 SCP communication
From: Jungo Lin @ 2019-07-26  8:07 UTC (permalink / raw)
  To: Tomasz Figa
  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, list
In-Reply-To: <CAAFQd5BT7M425AbFicYuX+wr-twgS_cxQ937+Rgxo6Y2fA6_gA@mail.gmail.com>

Hi, Tomasz:

On Thu, 2019-07-25 at 19:56 +0900, Tomasz Figa wrote:
> Hi Jungo,
> 
> On Sun, Jul 21, 2019 at 11:18 AM Jungo Lin <jungo.lin@mediatek.com> wrote:
> [snip]
> > > > +           wake_up_interruptible(&isp_ctx->composer_tx_thread.wq);
> > > > +           isp_ctx->composer_tx_thread.thread = NULL;
> > > > +   }
> > > > +
> > > > +   if (isp_ctx->composer_deinit_thread.thread) {
> > > > +           wake_up(&isp_ctx->composer_deinit_thread.wq);
> > > > +           isp_ctx->composer_deinit_thread.thread = NULL;
> > > > +   }
> > > > +   mutex_unlock(&isp_ctx->lock);
> > > > +
> > > > +   pm_runtime_put_sync(&p1_dev->pdev->dev);
> > >
> > > No need to use the sync variant.
> > >
> >
> > We don't get this point. If we will call pm_runtime_get_sync in
> > mtk_isp_hw_init function, will we need to call
> > pm_runtime_put_sync_autosuspend in mtk_isp_hw_release in next patch?
> > As we know, we should call runtime pm functions in pair.
> >
> 
> My point is that pm_runtime_put_sync() is only needed if one wants the
> runtime count to be decremented after the function returns. Normally
> there is no need to do so and one would call pm_runtime_put(), or if
> autosuspend is used, pm_runtime_put_autosuspend() (note there is no
> "sync" in the name).
> 
> [snip]

Ok, got your point.
We will change to use pm_runtime_put_autosuspend() which has ASYNC flag.

> > > +static void isp_composer_handler(void *data, unsigned int len, void *priv)
> > > > +{
> > > > +   struct mtk_isp_p1_ctx *isp_ctx = (struct mtk_isp_p1_ctx *)priv;
> > > > +   struct isp_p1_device *p1_dev = p1_ctx_to_dev(isp_ctx);
> > > > +   struct device *dev = &p1_dev->pdev->dev;
> > > > +   struct mtk_isp_scp_p1_cmd *ipi_msg;
> > > > +
> > > > +   ipi_msg = (struct mtk_isp_scp_p1_cmd *)data;
> > >
> > > Should we check that len == sizeof(*ipi_msg)? (Or at least >=, if data could
> > > contain some extra bytes at the end.)
> > >
> >
> > The len parameter is the actual sending bytes from SCP to kernel.
> > In the runtime, it is only 6 bytes for isp_ack_info command
> > However, sizeof(*ipi_msg) is large due to struct mtk_isp_scp_p1_cmd is
> > union structure.
> >
> 
> That said we still should check if len is enough to cover the data
> we're accessing below.
> 

Ok, we will add the len checking before accessing the data.

> > > > +
> > > > +   if (ipi_msg->cmd_id != ISP_CMD_ACK)
> > > > +           return;
> > > > +
> > > > +   if (ipi_msg->ack_info.cmd_id == ISP_CMD_FRAME_ACK) {
> > > > +           dev_dbg(dev, "ack frame_num:%d",
> > > > +                   ipi_msg->ack_info.frame_seq_no);
> > > > +           atomic_set(&isp_ctx->composed_frame_id,
> > > > +                      ipi_msg->ack_info.frame_seq_no);
> > >
> > > I suppose we are expecting here that ipi_msg->ack_info.frame_seq_no would be
> > > just isp_ctx->composed_frame_id + 1, right? If not, we probably dropped some
> > > frames and we should handle that somehow.
> > >
> >
> > No, we use isp_ctx->composed_frame_id to save which frame sequence
> > number are composed done in SCP. In new design, we will move this from
> > isp_ctx to p1_dev.
> 
> But we compose the frames in order, don't we? Wouldn't every composed
> frame would be just previous frame ID + 1?
> 
> [snip]

Yes, we compose the frames in order.
At the same time, we already increased "frame ID + 1" in
mtk_isp_req_enqueue() for each new request before sending to SCP for
composing. After receiving the ACK from SCP, we think the frame ID is
composed done and save by isp_ctx->composed_frame_id(v3).

[RFC v3]
void mtk_isp_req_enqueue(struct device *dev, struct media_request *req)
{
	...
	frameparams.frame_seq_no = isp_ctx->frame_seq_no++;

[RFC v4]
void mtk_isp_req_enqueue(struct mtk_cam_dev *cam,
			 struct mtk_cam_dev_request *req)
{
	struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(cam->dev);

	/* Accumulated frame sequence number */
	req->frame_params.frame_seq_no = ++p1_dev->enqueue_frame_seq_no;

 

> > > > +void isp_composer_hw_init(struct device *dev)
> > > > +{
> > > > +   struct mtk_isp_scp_p1_cmd composer_tx_cmd;
> > > > +   struct isp_p1_device *p1_dev = get_p1_device(dev);
> > > > +   struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> > > > +
> > > > +   memset(&composer_tx_cmd, 0, sizeof(composer_tx_cmd));
> > > > +   composer_tx_cmd.cmd_id = ISP_CMD_INIT;
> > > > +   composer_tx_cmd.frameparam.hw_module = isp_ctx->isp_hw_module;
> > > > +   composer_tx_cmd.frameparam.cq_addr.iova = isp_ctx->scp_mem_iova;
> > > > +   composer_tx_cmd.frameparam.cq_addr.scp_addr = isp_ctx->scp_mem_pa;
> > >
> > > Should we also specify the size of the buffer? Otherwise we could end up
> > > with some undetectable overruns.
> > >
> >
> > The size of SCP composer's memory is fixed to 0x200000.
> > Is it necessary to specify the size of this buffer?
> >
> > #define MTK_ISP_COMPOSER_MEM_SIZE 0x200000
> >
> > ptr = dma_alloc_coherent(p1_dev->cam_dev.smem_dev,
> >                         MTK_ISP_COMPOSER_MEM_SIZE, &addr, GFP_KERNEL);
> >
> 
> Okay, but please add a comment saying that this is an implicit
> requirement of the firmware.
> 
> Best regards,
> Tomasz

Ok, we will add comments.

Best regards,


Jungo

^ permalink raw reply

* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Georgi Djakov @ 2019-07-26  8:05 UTC (permalink / raw)
  To: Artur Świgoń, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-pm, dri-devel
  Cc: sw0312.kim, krzk, inki.dae, cw00.choi, myungjoo.ham, m.szyprowski
In-Reply-To: <20190723122016.30279-10-a.swigon@partner.samsung.com>

Hi Artur,

On 7/23/19 15:20, Artur Świgoń wrote:
> This patch adds interconnect functionality to the exynos-bus devfreq
> driver.
> 
> The SoC topology is a graph (or, more specifically, a tree) and most of its
> edges are taken from the devfreq parent-child hierarchy (cf.
> Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
> patch adds missing edges to the DT (under the name 'parent'). Due to
> unspecified relative probing order, -EPROBE_DEFER may be propagated to
> guarantee that a child is probed before its parent.
> 
> Each bus is now an interconnect provider and an interconnect node as well
> (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers
> itself as a node. Node IDs are not hardcoded but rather assigned at
> runtime, in probing order (subject to the above-mentioned exception
> regarding relative order). This approach allows for using this driver with
> various Exynos SoCs.

I am not familiar with the Exynos bus topology, but it seems to me that it's not
represented correctly. An interconnect provider with just a single node (port)
is odd. I would expect that each provider consists of multiple master and slave
nodes. This data would be used by a framework to understand what are the links
and how the traffic flows between the IP blocks and through which buses.

> The devfreq target() callback provided by exynos-bus now selects either the
> frequency calculated by the devfreq governor or the frequency requested via
> the interconnect API for the given node, whichever is higher.

This completely makes sense. We just need to be sure that the interconnect
framework is used correctly.

Thanks,
Georgi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 02/14] dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other nvmem based qcom socs
From: Viresh Kumar @ 2019-07-26  8:04 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Andy Gross, Ilia Lin,
	linux-arm-msm, jorge.ramirez-ortiz, bjorn.andersson, ulf.hansson,
	Sricharan R, Rob Herring, Rob Herring, Mark Rutland, linux-pm,
	devicetree, linux-kernel
In-Reply-To: <20190725104144.22924-3-niklas.cassel@linaro.org>

$subject of this and few more binding patches updated to:

"dt-bindings: opp: XXX"

On 25-07-19, 12:41, Niklas Cassel wrote:
> From: Sricharan R <sricharan@codeaurora.org>
> 
> The kryo cpufreq driver reads the nvmem cell and uses that data to
> populate the opps. There are other qcom cpufreq socs like krait which
> does similar thing. Except for the interpretation of the read data,
> rest of the driver is same for both the cases. So pull the common things
> out for reuse.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> [niklas.cassel@linaro.org: split dt-binding into a separate patch and
> do not rename the compatible string.]
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> Reviewed-by: Ilia Lin <ilia.lin@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> Changes since V1:
> -Picked up tags.

-- 
viresh

^ permalink raw reply

* Re: [PATCH 04/17] ARM: dts: vesnin: Add unit address for memory node
From: Alexander A. Filippov @ 2019-07-26  7:56 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: mark.rutland, devicetree, linux-aspeed, linux-kernel,
	Alexander Filippov, robh+dt, joel, linux-arm-kernel
In-Reply-To: <20190726053959.2003-5-andrew@aj.id.au>

On Fri, Jul 26, 2019 at 03:09:46PM +0930, Andrew Jeffery wrote:
> Fixes the following warnings:
> 
>     arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dt.yaml: /: memory: False schema does not allow {'reg': [[1073741824, 536870912]]}
>     arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dt.yaml: memory: 'device_type' is a required property
> 
> Cc: Alexander Filippov <a.filippov@yadro.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
> index 0b9e29c3212e..81d9dcb752a0 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
> @@ -14,7 +14,7 @@
>  		bootargs = "console=ttyS4,115200 earlyprintk";
>  	};
>  
> -	memory {
> +	memory@40000000 {
>  		reg = <0x40000000 0x20000000>;
>  	};
>  
> -- 
> 2.20.1
> 

Tested-by: Alexander Filippov <a.filippov@yadro.com>

^ permalink raw reply

* Re: [RFC, v3 9/9] media: platform: Add Mediatek ISP P1 shared memory device
From: Tomasz Figa @ 2019-07-26  7:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: devicetree, Sean Cheng (鄭昇弘),
	Mauro Carvalho Chehab, Rynn Wu (吳育恩),
	Linux Media Mailing List, srv_heupstream, Rob Herring,
	Ryan Yu (余孟修),
	Frankie Chiu (邱文凱), Hans Verkuil, Jungo Lin,
	Sj Huang, moderated list:ARM/Mediatek SoC support,
	Laurent Pinchart, ddavenport,
	Frederic Chen (陳俊元), list
In-Reply-To: <20190726074116.GA19745@infradead.org>

On Fri, Jul 26, 2019 at 4:41 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Jul 26, 2019 at 02:15:14PM +0900, Tomasz Figa wrote:
> > Could you try dma_get_sgtable() with the SCP struct device and then
> > dma_map_sg() with the P1 struct device?
>
> Please don't do that.  dma_get_sgtable is a pretty broken API (see
> the common near the arm implementation) and we should not add more
> users of it.  If you want a piece of memory that can be mapped to
> multiple devices allocate it using alloc_pages and then just map
> it to each device.

Thanks for taking a look at this thread.

Unfortunately that wouldn't work. We have a specific reserved memory
pool that is the only memory area accessible to one of the devices.
Any idea how to handle this?

Best regards,
Tomasz

^ permalink raw reply

* Re: [RFC, v3 9/9] media: platform: Add Mediatek ISP P1 shared memory device
From: Christoph Hellwig @ 2019-07-26  7:41 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	Sean Cheng (鄭昇弘), Mauro Carvalho Chehab,
	Rynn Wu (吳育恩), Linux Media Mailing List,
	srv_heupstream, Rob Herring, Ryan Yu (余孟修),
	Frankie Chiu (邱文凱), Hans Verkuil, Jungo Lin,
	Sj Huang, moderated list:ARM/Mediatek SoC support,
	Laurent Pinchart, ddavenport-F7+t8E8rja9g9hUCZPvPmw,
	Frederic Chen (陳俊元), list-Y9sIeH5OGRo
In-Reply-To: <CAAFQd5Bh80N+cMhz=eyHUGJLaE5uuypOawQvHrTgGSMDvmcpLA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Jul 26, 2019 at 02:15:14PM +0900, Tomasz Figa wrote:
> Could you try dma_get_sgtable() with the SCP struct device and then
> dma_map_sg() with the P1 struct device?

Please don't do that.  dma_get_sgtable is a pretty broken API (see
the common near the arm implementation) and we should not add more
users of it.  If you want a piece of memory that can be mapped to
multiple devices allocate it using alloc_pages and then just map
it to each device.

^ permalink raw reply

* Re: [RFC,v3 7/9] media: platform: Add Mediatek ISP P1 device driver
From: Jungo Lin @ 2019-07-26  7:23 UTC (permalink / raw)
  To: Tomasz Figa
  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, list
In-Reply-To: <CAAFQd5AmAXm1T0eLPiUTFLv9qiKxe-wZGyXxk0jvge+EanZ3FQ@mail.gmail.com>

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:
> > > > This patch adds the Mediatek ISP P1 HW control device driver.
> > > > It handles the ISP HW configuration, provides interrupt handling and
> > > > initializes the V4L2 device nodes and other functions.
> > > >
> > > > (The current metadata interface used in meta input and partial
> > > > meta nodes is only a temporary solution to kick off the driver
> > > > development and is not ready to be reviewed yet.)
> > > >
> > > > Signed-off-by: Jungo Lin <jungo.lin@mediatek.com>
> > > > ---
> > > >  .../platform/mtk-isp/isp_50/cam/Makefile      |    1 +
> > > >  .../mtk-isp/isp_50/cam/mtk_cam-regs.h         |  126 ++
> > > >  .../platform/mtk-isp/isp_50/cam/mtk_cam.c     | 1087 +++++++++++++++++
> > > >  .../platform/mtk-isp/isp_50/cam/mtk_cam.h     |  243 ++++
> > > >  4 files changed, 1457 insertions(+)
> > > >  create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-regs.h
> > > >  create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c
> > > >  create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h
> > > >
> > >
> > > Thanks for the patch! Please see my comments inline.
> > >
> > > [snip]
> > >
> >
> > Thanks for your comments. Please check my replies inline.
> >
> 
> Thanks! I'll snip anything I don't have further comments on.
> 
> [snip]

Ok, got it.

> > > > +/* META */
> > > > +#define REG_META0_VB2_INDEX                0x14dc
> > > > +#define REG_META1_VB2_INDEX                0x151c
> > >
> > > I don't believe these registers are really for VB2 indexes.
> > >
> >
> > MTK P1 ISP HW supports frame header spare registers for each DMA, such
> > as CAM_DMA_FH_AAO_SPARE or CAM_DMA_FH_AFO_SPARE. We could save some
> > frame information in these ISP registers. In this case, we save META0
> > VB2 index into AAO FH spare register and META1 VB2 index into AFO FH
> > spare register. These implementation is designed for non-request 3A
> > DMAs. These VB2 indexes are sent in ISP_CMD_ENQUEUE_META command of
> > mtk_isp_enqueue function. So we just call CAM_DMA_FH_AAO_SPARE as
> > REG_META0_VB2_INDEX for easy understanding.
> 
> Unfortunately it's not a good idea to mix hardware concepts with
> naming specific to the OS the driver is written for. Better to keep
> the hardware naming, e.g. CAM_DMA_FH_AAO_SPARE.
> 

Ok, got your point. We will pay attention in next time.
Moreover, we will remove AAO/AFO non-request design in next patch.
So these codes will also be removed.

> > Moreover, if we only need to
> > support request mode, we should remove this here.
> >
> > cmd_params.cmd_id = ISP_CMD_ENQUEUE_META;
> > cmd_params.meta_frame.enabled_dma = dma_port;
> > cmd_params.meta_frame.vb_index = buffer->vbb.vb2_buf.index;
> > cmd_params.meta_frame.meta_addr.iova = buffer->daddr;
> > cmd_params.meta_frame.meta_addr.scp_addr = buffer->scp_addr;
> >
> 
> Okay, removing sounds good to me. Let's keep the code simple.
> 
> [snip]

Thanks.

> > > > +
> > > > +   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;
}

> >
> > 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]?

[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.


> > - 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.

> > - 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.
f. The CQ buffer loading is guaranteed by HW to finish before the next
SOF.

> >
> > Btw, we will revise the above implementation as below.
> >
> >
> > if (irq_status & SOF_INT_ST)
> >         mtk_cam_dev_event_frame_sync(&p1_dev->cam_dev,
> >                                              dequeue_frame_seq_no);
> >
> > /* Sof, done order check */
> > 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);
> >
> > /* Notify IRQ event and de-enqueue frame */
> > irq_handle_notify_event(p1_dev, irq_status, dma_status);
> 
> Don't we still need to do this conditionally, only if we got HW_PASS1_DON_ST?
> 
> [snip]

Yes, in the new version, we will add SW_PASS1_DON_ST check before
calling mtk_cam_dev_dequeue_req_frame function.

> > > > +/* ISP P1 interface functions */
> > > > +int mtk_isp_power_init(struct mtk_cam_dev *cam_dev)
> > > > +{
> > > > +   struct device *dev = &cam_dev->pdev->dev;
> > > > +   struct isp_p1_device *p1_dev = get_p1_device(dev);
> > > > +   struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> > > > +   int ret;
> > > > +
> > > > +   ret = isp_setup_scp_rproc(p1_dev);
> > > > +   if (ret)
> > > > +           return ret;
> > > > +
> > > > +   ret = isp_init_context(p1_dev);
> > > > +   if (ret)
> > > > +           return ret;
> > >
> > > The above function doesn't really seem to be related to power management.
> > > Should it be called from subdev stream on?
> > >
> >
> > We will rename this function to mtk_isp_hw_init.
> > But, it will be called when the first video node is streamed on.
> > This is because we need to initialize the HW firstly for sub-device
> > stream-on performance.  We need to send some IPI commands, such as
> > ISP_CMD_INIT & ISP_CMD_CONFIG_META & ISP_CMD_ENQUEUE_META in this
> > timing.
> 
> What performance do you mean here? The time between first video node
> stream on and last video node stream on should be really short. Are
> you seeing some long delays there?
> 
> That said, doing it when the first video node starts streaming is okay.
> 
> [snip]

Ok, for IPI command sending performance issue is gone with removing the
design of non-request mode of 3A buffers. We could skip this question.

> > > > +   /* Use pure RAW as default HW path */
> > > > +   isp_ctx->isp_raw_path = ISP_PURE_RAW_PATH;
> > > > +   atomic_set(&p1_dev->cam_dev.streamed_node_count, 0);
> > > > +
> > > > +   isp_composer_hw_init(dev);
> > > > +   /* Check enabled DMAs which is configured by media setup */
> > > > +   isp_composer_meta_config(dev, get_enabled_dma_ports(cam_dev));
> > >
> > > Hmm, this seems to be also configured by isp_compoer_hw_config(). Are both
> > > necessary?
> > >
> >
> > Yes, it is necessary for non-request buffers design for Camera 3A video
> > nodes. For 3A video nodes, we just want to know which 3A video nodes are
> > enabled in ISP_CMD_CONFIG_META. In this stage, we may not know the image
> > format from user space. So we just pass the enabled DMA information from
> > kernel to SCP only. With 3A enabled DMA information, we could configure
> > related 3A registers in SCP.
> 
> We should try to remove this non-request mode. Let's continue
> discussion on the other patch where I brought this topic.
> 
> [snip]

Ok. We will remove the non-request in next patch.

> > > > +int mtk_isp_power_release(struct device *dev)
> > > > +{
> > > > +   isp_composer_hw_deinit(dev);
> > > > +   isp_uninit_context(dev);
> > >
> > > These two don't seem to be related to power either.
> > >
> > > Instead, I don't see anything that could undo the rproc_boot() operation
> > > here.
> > >
> >
> > We will rename this function to mtk_isp_hw_release.
> > We will also add rproc_shutdown function call here.
> >
> > int mtk_isp_hw_release(struct mtk_cam_dev *cam)
> > {
> >         struct device *dev = cam->dev;
> >         struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(dev);
> >
> >         isp_composer_hw_deinit(p1_dev);
> >         pm_runtime_put_sync_autosuspend(dev);
> 
> Note that for autosuspend to work correctly, you also need to call
> pm_runtime_mark_last_busy() before this one.
> 
> [snip]

Ok, thanks for your hint.

> > > > +   struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> > > > +   struct p1_frame_param frameparams;
> > > > +   struct mtk_isp_queue_job *framejob;
> > > > +   struct media_request_object *obj, *obj_safe;
> > > > +   struct vb2_buffer *vb;
> > > > +   struct mtk_cam_dev_buffer *buf;
> > > > +
> > > > +   framejob = kzalloc(sizeof(*framejob), GFP_ATOMIC);
> > >
> > > This allocation shouldn't be needed. The structure should be already a part
> > > of the mtk_cam_dev_request struct that wraps media_request. Actually. I'd
> > > even say that the contents of this struct should be just moved to that one
> > > to avoid overabstracting.
> > >
> >
> > For this function, we will apply the new design from P2 driver's
> > comment. Here is the new implementation.
> >
> > struct mtk_cam_dev_request {
> >         struct media_request req;
> >         struct mtk_p1_frame_param frame_params;
> >         struct work_struct frame_work;
> >         struct list_head list;
> >         atomic_t buf_count;
> > };
> >
> > void mtk_isp_req_enqueue(struct mtk_cam_dev *cam,
> >                          struct mtk_cam_dev_request *req)
> > {
> >         struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(cam->dev);
> >         int ret;
> >
> >         req->frame_params.frame_seq_no = ++p1_dev->enqueue_frame_seq_no;
> >         INIT_WORK(&req->frame_work, isp_tx_frame_worker);
> >         ret = queue_work(p1_dev->composer_wq, &req->frame_work);
> >         if (!ret)
> >                 dev_dbg(cam->dev, "frame_no:%d queue_work failed\n",
> >                         req->frame_params.frame_seq_no, ret);
> >         else
> >                 dev_dbg(cam->dev, "Enqueue fd:%s frame_seq_no:%d job cnt:%d\n",
> >                         req->req.debug_str, req->frame_params.frame_seq_no,
> >                         atomic_read(&cam->running_job_count));
> 
> It shouldn't be possible for queue_work() to fail here. We just
> received a brand new request from the Request API core and called
> INIT_WORK() on the work struct.
> 
> [snip]

Ok, got it. We will remove this unnecessary error checking.

> > > > +   enable_sys_clock(p1_dev);
> > > > +
> > > > +   /* V4L2 stream-on phase & restore HW stream-on status */
> > > > +   if (p1_dev->cam_dev.streaming) {
> > > > +           dev_dbg(dev, "Cam:%d resume,enable VF\n", module);
> > > > +           /* Enable CMOS */
> > > > +           reg_val = readl(isp_dev->regs + REG_TG_SEN_MODE);
> > > > +           writel((reg_val | CMOS_EN_BIT),
> > > > +                  isp_dev->regs + REG_TG_SEN_MODE);
> > > > +           /* Enable VF */
> > > > +           reg_val = readl(isp_dev->regs + REG_TG_VF_CON);
> > > > +           writel((reg_val | VFDATA_EN_BIT),
> > > > +                  isp_dev->regs + REG_TG_VF_CON);
> > > > +   }
> > >
> > > Does the hardware keep all the state, e.g. queued buffers, during suspend?
> > > Would the code above continue all the capture from the next buffer, as
> > > queued by the userspace before the suspend?
> > >
> >
> > Yes, we will test it.
> > 1. SCP buffers are kept by SCP processor
> > 2. ISP registers are still kept even if ISP clock is disable.
> >
> 
> That said, during system suspend, it would be more than just ISP clock
> disabled. I'd expect that the ISP power domain would be powered off.
> However, if we ensure that the ISP completes before suspend, I guess
> that after the resume the next frame CQ buffer would reprogram all the
> registers, right?
> 
> Also, would SCP always keep running in system suspend?
> 
> [snip]

Q1. Firstly, we will make sure the ISP complete the current frame before
suspend. For the resume behavior, we will check internally.

Q2. Sorry for my wrong reply. The CQ buffer data should be kept in
memory, not SCP. SCP should also be stopped when system is suspend.

> > > > +
> > > > +   for (i = ISP_CAMSYS_CONFIG_IDX; i < ISP_DEV_NODE_NUM; i++) {
> > >
> > > I think we want to start from 0 here?
> > >
> >
> > Because of single CAM support, we will revise our DTS tree to support
> > single CAM only.
> 
> Note that DT bindings should describe the hardware not the driver. So
> please design the bindings in a way that would cover all the cameras,
> even if the driver only takes the information needed to handle 1.
> 

Ok, we will cover all Camera ISP P1 HW units in the DT.

> > So we could remove this loop and check the CAM-B HW
> > information here. Here is below new function.
> >
> > static int mtk_isp_probe(struct platform_device *pdev)
> > {
> >         /* List of clocks required by isp cam */
> >         static const char * const clk_names[] = {
> >                 "camsys_cam_cgpdn", "camsys_camtg_cgpdn"
> >         };
> >         struct mtk_isp_p1_device *p1_dev;
> >         struct device *dev = &pdev->dev;
> >         struct resource *res;
> >         int irq, ret, i;
> >
> >         p1_dev = devm_kzalloc(dev, sizeof(*p1_dev), GFP_KERNEL);
> >         if (!p1_dev)
> >                 return -ENOMEM;
> >
> >         p1_dev->dev = dev;
> >         dev_set_drvdata(dev, p1_dev);
> >
> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >         p1_dev->regs = devm_ioremap_resource(dev, res);
> >         if (IS_ERR(p1_dev->regs)) {
> >                 dev_err(dev, "Failed platform resources map\n");
> >                 return PTR_ERR(p1_dev->regs);
> >         }
> >         dev_dbg(dev, "cam, map_addr=0x%pK\n", p1_dev->regs);
> >
> >         irq = platform_get_irq(pdev, 0);
> >         if (!irq) {
> >                 dev_err(dev, "Missing IRQ resources data\n");
> >                 return -ENODEV;
> >         }
> >         ret = devm_request_irq(dev, irq, isp_irq_cam, IRQF_SHARED,
> >                                dev_name(dev), p1_dev);
> >         if (ret) {
> >                 dev_err(dev, "req_irq fail, dev:%s irq=%d\n",
> >                         dev->of_node->name, irq);
> >                 return ret;
> >         }
> >         dev_dbg(dev, "Reg. irq=%d, isr:%s\n", irq, dev_driver_string(dev));
> >         spin_lock_init(&p1_dev->spinlock_irq);
> >
> >         p1_dev->num_clks = ARRAY_SIZE(clk_names);
> >         p1_dev->clks = devm_kcalloc(dev, p1_dev->num_clks,
> >                                     sizeof(*p1_dev->clks), GFP_KERNEL);
> >         if (!p1_dev->clks)
> >                 return -ENOMEM;
> >
> >         for (i = 0; i < p1_dev->num_clks; ++i)
> >                 p1_dev->clks[i].id = clk_names[i];
> >
> >         ret = devm_clk_bulk_get(dev, p1_dev->num_clks, p1_dev->clks);
> >         if (ret) {
> >                 dev_err(dev, "cannot get isp cam clock:%d\n", ret);
> >                 return ret;
> >         }
> >
> >         ret = isp_setup_scp_rproc(p1_dev, pdev);
> >         if (ret)
> >                 return ret;
> >
> >         pm_runtime_enable(dev);
> 
> We also need to call pm_runtime_use_autosuspend() and
> pm_runtime_set_autosuspend_delay() before enabling runtime PM. I'd
> suggest an autosuspend delay equal to around 2x the time that's needed
> to stop and start streaming in total.
> 
> [snip]

O, we will add these function calls.

> > > > +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;

	/* 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;

	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;

	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;

	dev_dbg(dev, "enable clock\n");
	ret = clk_bulk_prepare_enable(p1_dev->num_clks, p1_dev->clks);
	if (ret) {
		dev_err(dev, "cannot enable clock:%d\n", ret);
		return ret;
	}

	return 0;
}

static const struct dev_pm_ops mtk_isp_pm_ops = {
	SET_SYSTEM_SLEEP_PM_OPS(mtk_isp_pm_suspend, mtk_isp_pm_resume)
	SET_RUNTIME_PM_OPS(mtk_isp_runtime_suspend, mtk_isp_runtime_resume,
			   NULL)
};

Best regards,


Jungo

^ permalink raw reply

* [PATCH 6/6] dt-bindings: spi: sprd: Change the hwlock support to be optional
From: Baolin Wang @ 2019-07-26  7:20 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, orsonzhai, zhang.lyra
  Cc: weicx, sherry.zong, baolin.wang, vincent.guittot, linux-spi,
	devicetree, linux-kernel
In-Reply-To: <cover.1564125131.git.baolin.wang@linaro.org>

No need to add hardware spinlock proctection due to add multiple
msater channel, so change it to be optional in documentation.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 .../devicetree/bindings/spi/spi-sprd-adi.txt       |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-sprd-adi.txt b/Documentation/devicetree/bindings/spi/spi-sprd-adi.txt
index 8de589b..2567c82 100644
--- a/Documentation/devicetree/bindings/spi/spi-sprd-adi.txt
+++ b/Documentation/devicetree/bindings/spi/spi-sprd-adi.txt
@@ -25,18 +25,23 @@ data by ADI software channels at the same time, or two parallel routine of setti
 ADI registers will make ADI controller registers chaos to lead incorrect results.
 Then we need one hardware spinlock to synchronize between the multiple subsystems.
 
+The new version ADI controller supplies multiple master channels for different
+subsystem accessing, that means no need to add hardware spinlock to synchronize,
+thus change the hardware spinlock support to be optional to keep backward
+compatibility.
+
 Required properties:
 - compatible: Should be "sprd,sc9860-adi".
 - reg: Offset and length of ADI-SPI controller register space.
-- hwlocks: Reference to a phandle of a hwlock provider node.
-- hwlock-names: Reference to hwlock name strings defined in the same order
-	as the hwlocks, should be "adi".
 - #address-cells: Number of cells required to define a chip select address
 	on the ADI-SPI bus. Should be set to 1.
 - #size-cells: Size of cells required to define a chip select address size
 	on the ADI-SPI bus. Should be set to 0.
 
 Optional properties:
+- hwlocks: Reference to a phandle of a hwlock provider node.
+- hwlock-names: Reference to hwlock name strings defined in the same order
+	as the hwlocks, should be "adi".
 - sprd,hw-channels: This is an array of channel values up to 49 channels.
 	The first value specifies the hardware channel id which is used to
 	transfer data triggered by hardware automatically, and the second
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 5/6] spi: sprd: adi: Change hwlock to be optional
From: Baolin Wang @ 2019-07-26  7:20 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, orsonzhai, zhang.lyra
  Cc: weicx, sherry.zong, baolin.wang, vincent.guittot, linux-spi,
	devicetree, linux-kernel
In-Reply-To: <cover.1564125131.git.baolin.wang@linaro.org>

Now Spreadtrum ADI controller supplies multiple master accessing channel
to support multiple subsystems accessing, instead of using a hardware
spinlock to synchronize between the multiple subsystems.

To keep backward compatibility, we should change the hardware spinlock
to be optional. Moreover change to use of_hwspin_lock_get_id() function
which return -ENOENT error number to indicate no hwlock support.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/spi/spi-sprd-adi.c |   68 +++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 26 deletions(-)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index 0d767eb..9a05128 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -165,14 +165,16 @@ static int sprd_adi_read(struct sprd_adi *sadi, u32 reg_paddr, u32 *read_val)
 	int read_timeout = ADI_READ_TIMEOUT;
 	unsigned long flags;
 	u32 val, rd_addr;
-	int ret;
-
-	ret = hwspin_lock_timeout_irqsave(sadi->hwlock,
-					  ADI_HWSPINLOCK_TIMEOUT,
-					  &flags);
-	if (ret) {
-		dev_err(sadi->dev, "get the hw lock failed\n");
-		return ret;
+	int ret = 0;
+
+	if (sadi->hwlock) {
+		ret = hwspin_lock_timeout_irqsave(sadi->hwlock,
+						  ADI_HWSPINLOCK_TIMEOUT,
+						  &flags);
+		if (ret) {
+			dev_err(sadi->dev, "get the hw lock failed\n");
+			return ret;
+		}
 	}
 
 	/*
@@ -219,7 +221,8 @@ static int sprd_adi_read(struct sprd_adi *sadi, u32 reg_paddr, u32 *read_val)
 	*read_val = val & RD_VALUE_MASK;
 
 out:
-	hwspin_unlock_irqrestore(sadi->hwlock, &flags);
+	if (sadi->hwlock)
+		hwspin_unlock_irqrestore(sadi->hwlock, &flags);
 	return ret;
 }
 
@@ -230,12 +233,14 @@ static int sprd_adi_write(struct sprd_adi *sadi, u32 reg_paddr, u32 val)
 	unsigned long flags;
 	int ret;
 
-	ret = hwspin_lock_timeout_irqsave(sadi->hwlock,
-					  ADI_HWSPINLOCK_TIMEOUT,
-					  &flags);
-	if (ret) {
-		dev_err(sadi->dev, "get the hw lock failed\n");
-		return ret;
+	if (sadi->hwlock) {
+		ret = hwspin_lock_timeout_irqsave(sadi->hwlock,
+						  ADI_HWSPINLOCK_TIMEOUT,
+						  &flags);
+		if (ret) {
+			dev_err(sadi->dev, "get the hw lock failed\n");
+			return ret;
+		}
 	}
 
 	ret = sprd_adi_drain_fifo(sadi);
@@ -261,7 +266,8 @@ static int sprd_adi_write(struct sprd_adi *sadi, u32 reg_paddr, u32 val)
 	}
 
 out:
-	hwspin_unlock_irqrestore(sadi->hwlock, &flags);
+	if (sadi->hwlock)
+		hwspin_unlock_irqrestore(sadi->hwlock, &flags);
 	return ret;
 }
 
@@ -476,16 +482,26 @@ static int sprd_adi_probe(struct platform_device *pdev)
 	sadi->slave_pbase = res->start + ADI_SLAVE_OFFSET;
 	sadi->ctlr = ctlr;
 	sadi->dev = &pdev->dev;
-	ret = of_hwspin_lock_get_id_byname(np, "adi");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "can not get the hardware spinlock\n");
-		goto put_ctlr;
-	}
-
-	sadi->hwlock = devm_hwspin_lock_request_specific(&pdev->dev, ret);
-	if (!sadi->hwlock) {
-		ret = -ENXIO;
-		goto put_ctlr;
+	ret = of_hwspin_lock_get_id(np, 0);
+	if (ret > 0 || (IS_ENABLED(CONFIG_HWSPINLOCK) && ret == 0)) {
+		sadi->hwlock =
+			devm_hwspin_lock_request_specific(&pdev->dev, ret);
+		if (!sadi->hwlock) {
+			ret = -ENXIO;
+			goto put_ctlr;
+		}
+	} else {
+		switch (ret) {
+		case -ENOENT:
+			dev_info(&pdev->dev, "no hardware spinlock supplied\n");
+			break;
+		default:
+			dev_err(&pdev->dev,
+				"failed to find hwlock id, %d\n", ret);
+			/* fall-through */
+		case -EPROBE_DEFER:
+			goto put_ctlr;
+		}
 	}
 
 	sprd_adi_hw_init(sadi);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 4/6] spi: sprd: adi: Add a reset reason for watchdog mode
From: Baolin Wang @ 2019-07-26  7:20 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, orsonzhai, zhang.lyra
  Cc: weicx, sherry.zong, baolin.wang, vincent.guittot, linux-spi,
	devicetree, linux-kernel
In-Reply-To: <cover.1564125131.git.baolin.wang@linaro.org>

From: Sherry Zong <sherry.zong@unisoc.com>

When the system was rebooted by watchdog, now we did not save the watchdog
reset mode which will make system enter a incorrect mode after rebooting.

Thus we should set the watchdog reset mode as default when opening the
watchdog configuration, that means if the system was rebooted by other
reason through the restart_handler(), then we will clear the default
watchdog reset mode to save the correct reset mode.

Signed-off-by: Sherry Zong <sherry.zong@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/spi/spi-sprd-adi.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index 509ce69..0d767eb 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -99,6 +99,7 @@
 #define HWRST_STATUS_IQMODE		0xb0
 #define HWRST_STATUS_SPRDISK		0xc0
 #define HWRST_STATUS_FACTORYTEST	0xe0
+#define HWRST_STATUS_WATCHDOG		0xf0
 
 /* Use default timeout 50 ms that converts to watchdog values */
 #define WDG_LOAD_VAL			((50 * 1000) / 32768)
@@ -309,6 +310,18 @@ static int sprd_adi_transfer_one(struct spi_controller *ctlr,
 	return 0;
 }
 
+static void sprd_adi_set_wdt_rst_mode(struct sprd_adi *sadi)
+{
+#ifdef CONFIG_SPRD_WATCHDOG
+	u32 val;
+
+	/* Set default watchdog reboot mode */
+	sprd_adi_read(sadi, sadi->slave_pbase + PMIC_RST_STATUS, &val);
+	val |= HWRST_STATUS_WATCHDOG;
+	sprd_adi_write(sadi, sadi->slave_pbase + PMIC_RST_STATUS, val);
+#endif
+}
+
 static int sprd_adi_restart_handler(struct notifier_block *this,
 				    unsigned long mode, void *cmd)
 {
@@ -347,6 +360,7 @@ static int sprd_adi_restart_handler(struct notifier_block *this,
 
 	/* Record the reboot mode */
 	sprd_adi_read(sadi, sadi->slave_pbase + PMIC_RST_STATUS, &val);
+	val &= ~HWRST_STATUS_WATCHDOG;
 	val |= reboot_mode;
 	sprd_adi_write(sadi, sadi->slave_pbase + PMIC_RST_STATUS, val);
 
@@ -475,6 +489,7 @@ static int sprd_adi_probe(struct platform_device *pdev)
 	}
 
 	sprd_adi_hw_init(sadi);
+	sprd_adi_set_wdt_rst_mode(sadi);
 
 	ctlr->dev.of_node = pdev->dev.of_node;
 	ctlr->bus_num = pdev->id;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 3/6] spi: sprd: adi: Add a reset reason for factory test mode
From: Baolin Wang @ 2019-07-26  7:20 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, orsonzhai, zhang.lyra
  Cc: weicx, sherry.zong, baolin.wang, vincent.guittot, linux-spi,
	devicetree, linux-kernel
In-Reply-To: <cover.1564125131.git.baolin.wang@linaro.org>

From: Sherry Zong <sherry.zong@unisoc.com>

Add a new reset flag to indicate that the system need enter factory test
mode after restarting system.

Signed-off-by: Sherry Zong <sherry.zong@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/spi/spi-sprd-adi.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index 0b3f23a..509ce69 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -98,6 +98,7 @@
 #define HWRST_STATUS_AUTODLOADER	0xa0
 #define HWRST_STATUS_IQMODE		0xb0
 #define HWRST_STATUS_SPRDISK		0xc0
+#define HWRST_STATUS_FACTORYTEST	0xe0
 
 /* Use default timeout 50 ms that converts to watchdog values */
 #define WDG_LOAD_VAL			((50 * 1000) / 32768)
@@ -339,6 +340,8 @@ static int sprd_adi_restart_handler(struct notifier_block *this,
 		reboot_mode = HWRST_STATUS_SPRDISK;
 	else if (!strncmp(cmd, "tospanic", 8))
 		reboot_mode = HWRST_STATUS_SECURITY;
+	else if (!strncmp(cmd, "factorytest", 11))
+		reboot_mode = HWRST_STATUS_FACTORYTEST;
 	else
 		reboot_mode = HWRST_STATUS_NORMAL;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 2/6] spi: sprd: adi: Add a reset reason for TOS panic
From: Baolin Wang @ 2019-07-26  7:20 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, orsonzhai, zhang.lyra
  Cc: weicx, sherry.zong, baolin.wang, vincent.guittot, linux-spi,
	devicetree, linux-kernel
In-Reply-To: <cover.1564125131.git.baolin.wang@linaro.org>

From: Chenxu Wei <weicx@spreadst.com>

Add a new reset flag to indicate the reset reason is caused by TOS.

Signed-off-by: Chenxu Wei <weicx@spreadst.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/spi/spi-sprd-adi.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index 11880db..0b3f23a 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -86,6 +86,7 @@
 #define BIT_WDG_EN			BIT(2)
 
 /* Definition of PMIC reset status register */
+#define HWRST_STATUS_SECURITY		0x02
 #define HWRST_STATUS_RECOVERY		0x20
 #define HWRST_STATUS_NORMAL		0x40
 #define HWRST_STATUS_ALARM		0x50
@@ -336,6 +337,8 @@ static int sprd_adi_restart_handler(struct notifier_block *this,
 		reboot_mode = HWRST_STATUS_IQMODE;
 	else if (!strncmp(cmd, "sprdisk", 7))
 		reboot_mode = HWRST_STATUS_SPRDISK;
+	else if (!strncmp(cmd, "tospanic", 8))
+		reboot_mode = HWRST_STATUS_SECURITY;
 	else
 		reboot_mode = HWRST_STATUS_NORMAL;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 1/6] spi: sprd: adi: Remove redundant address bits setting
From: Baolin Wang @ 2019-07-26  7:20 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, orsonzhai, zhang.lyra
  Cc: weicx, sherry.zong, baolin.wang, vincent.guittot, linux-spi,
	devicetree, linux-kernel
In-Reply-To: <cover.1564125131.git.baolin.wang@linaro.org>

The ADI default transfer address bits is 12bit on Spreadtrum SC9860
platform, thus there is no need to set again, remove it.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/spi/spi-sprd-adi.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index df5960b..11880db 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -380,9 +380,6 @@ static void sprd_adi_hw_init(struct sprd_adi *sadi)
 	const __be32 *list;
 	u32 tmp;
 
-	/* Address bits select default 12 bits */
-	writel_relaxed(0, sadi->base + REG_ADI_CTRL0);
-
 	/* Set all channels as default priority */
 	writel_relaxed(0, sadi->base + REG_ADI_CHN_PRIL);
 	writel_relaxed(0, sadi->base + REG_ADI_CHN_PRIH);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 0/6] Optimize Spreadtrum ADI driver
From: Baolin Wang @ 2019-07-26  7:20 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, orsonzhai, zhang.lyra
  Cc: weicx, sherry.zong, baolin.wang, vincent.guittot, linux-spi,
	devicetree, linux-kernel

This patchset did some optimization to remove some redundant code,
add more reboot mode support and change hardware spinlock support
to be optional.

Baolin Wang (3):
  spi: sprd: adi: Remove redundant address bits setting
  spi: sprd: adi: Change hwlock to be optional
  dt-bindings: spi: sprd: Change the hwlock support to be optional

Chenxu Wei (1):
  spi: sprd: adi: Add a reset reason for TOS panic

Sherry Zong (2):
  spi: sprd: adi: Add a reset reason for factory test mode
  spi: sprd: adi: Add a reset reason for watchdog mode

 .../devicetree/bindings/spi/spi-sprd-adi.txt       |   11 ++-
 drivers/spi/spi-sprd-adi.c                         |   92 ++++++++++++++------
 2 files changed, 71 insertions(+), 32 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* Re: [PATCH net-next 3/3] dt-bindings: net: ethernet: Update mt7622 docs and dts to reflect the new phylink API
From: René van Dorst @ 2019-07-26  7:19 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, frank-w, sean.wang, f.fainelli, linux, davem,
	matthias.bgg, vivien.didelot, john, linux-mediatek, linux-mips,
	robh+dt, devicetree
In-Reply-To: <20190725193123.GA32542@lunn.ch>

Quoting Andrew Lunn <andrew@lunn.ch>:

>> +	gmac0: mac@0 {
>> +		compatible = "mediatek,eth-mac";
>> +		reg = <0>;
>> +		phy-mode = "sgmii";
>> +
>> +		fixed-link {
>> +			speed = <2500>;
>> +			full-duplex;
>> +			pause;
>> +		};
>> +	};
>
> Hi René
>

Hi Andrew,

> SGMII and fixed-link is rather odd. Why do you need this combination?

BananaPi R64 has a RTL8367S 5+2-port switch, switch interfaces with  
the SOC by a
(H)SGMII and/or RGMII interface. SGMII is mainly used for the LAN ports and
RGMII for the WAN port.

I mimic the SDK software which puts SGMII interface in 2.5GBit  
fixed-link mode.
The RTL8367S switch code also put switch mac in forge 2.5GBit mode.

So this is the reason why I put a fixed-link mode here.

Greats,

René

>       Andrew

^ permalink raw reply

* Re: [PATCH v2 2/2] mmc: Add support for the ASPEED SD controller
From: Andrew Jeffery @ 2019-07-26  6:47 UTC (permalink / raw)
  To: Adrian Hunter, linux-mmc
  Cc: mark.rutland, devicetree, Ulf Hansson, linux-aspeed, Ryan Chen,
	linux-kernel, Rob Herring, Joel Stanley, linux-arm-kernel
In-Reply-To: <8a7bfe52-83ca-7601-7d75-e5615da7b5de@intel.com>



On Fri, 26 Jul 2019, at 15:27, Adrian Hunter wrote:
> On 26/07/19 3:52 AM, Andrew Jeffery wrote:
> > On Thu, 25 Jul 2019, at 22:49, Adrian Hunter wrote:
> >> On 12/07/19 6:32 AM, Andrew Jeffery wrote:
> >>> +static int aspeed_sdhci_probe(struct platform_device *pdev)
> >>> +{
> >>> +	struct sdhci_pltfm_host *pltfm_host;
> >>> +	struct aspeed_sdhci *dev;
> >>> +	struct sdhci_host *host;
> >>> +	struct resource *res;
> >>> +	int slot;
> >>> +	int ret;
> >>> +
> >>> +	host = sdhci_pltfm_init(pdev, &aspeed_sdc_pdata, sizeof(*dev));
> >>> +	if (IS_ERR(host))
> >>> +		return PTR_ERR(host);
> >>> +
> >>> +	pltfm_host = sdhci_priv(host);
> >>> +	dev = sdhci_pltfm_priv(pltfm_host);
> >>> +	dev->parent = dev_get_drvdata(pdev->dev.parent);
> >>> +
> >>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>> +	slot = aspeed_sdhci_calculate_slot(dev, res);
> >>> +	if (slot < 0)
> >>> +		return slot;
> >>> +	dev_info(&pdev->dev, "Configuring for slot %d\n", slot);
> >>> +	dev->width_mask = !slot ? ASPEED_SDC_S0MMC8 : ASPEED_SDC_S1MMC8;
> >>
> >> That implies that you only support 2 slots which begs the question why
> >> you don't validate slot.
> > 
> > I'm not sure what you mean here, but I'll dig into it.
> 
> I just meant, if you only support 2 slots:
> 
> 	if (slot > 1)
> 		return -EINVAL;
>

Oh, sure.

^ permalink raw reply

* Re: [PATCH V6 17/21] arm64: tegra: Enable wake from deep sleep on RTC alarm.
From: Dmitry Osipenko @ 2019-07-26  6:30 UTC (permalink / raw)
  To: Sowjanya Komatineni
  Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
	linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
	sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
	linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <1563738060-30213-18-git-send-email-skomatineni@nvidia.com>

В Sun, 21 Jul 2019 12:40:56 -0700
Sowjanya Komatineni <skomatineni@nvidia.com> пишет:

> This patch updates device tree for RTC and PMC to allow system wake
> from deep sleep on RTC alarm.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>

The dot in the end of the commit's title is unnecessary.

^ permalink raw reply

* [PATCH v2 2/2] ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards
From: Krzysztof Kozlowski @ 2019-07-26  6:17 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-kernel, linux-arm-kernel, Schrempf Frieder
  Cc: Krzysztof Kozlowski
In-Reply-To: <20190726061705.14764-1-krzk@kernel.org>

Add support for i.MX6UL modules from Kontron Electronics GmbH (before
acquisition: Exceet Electronics) and evalkit boards based on it:

1. N6310 SOM: i.MX6 UL System-on-Module, a 25x25 mm solderable module
   (LGA pads and pin castellations) with 256 MB RAM, 1 MB NOR-Flash,
   256 MB NAND and other interfaces,
2. N6310 S: evalkit, w/wo eMMC, without display,
3. N6310 S 43: evalkit with 4.3" display,
4. N6310 S 50: evalkit with 5.0" display.

This includes device nodes for unsupported displays (Admatec
T043C004800272T2A and T070P133T0S301).

The work is based on Exceet/Kontron source code (GPLv2) with numerous
changes:
1. Reorganize files,
2. Rename Exceet -> Kontron,
3. Rename models/compatibles to match newest Kontron product naming,
4. Fix coding style errors,
5. Fix DTC warnings,
6. Extend compatibles so eval boards inherit the SoM compatible,
7. Use defines instead of GPIO flag values,
8. Use proper vendor compatible for Macronix SPI NOR,
9. Sort nodes alphabetically.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Changes since v1, after Frieder's review:
1. Remove unneeded license notes,
2. Add Kontron copyright (2018),
3. Rename the files/models/compatibles to new naming - N6310,
4. Remove unneeded CPU operating points override,
5. Switch regulator nodes into simple children nodes without addresses
   (so not simple bus),
6. Use proper vendor compatible for Macronix SPI NOR.
---
 .../devicetree/bindings/arm/fsl.yaml          |   4 +
 arch/arm/boot/dts/Makefile                    |   3 +
 .../boot/dts/imx6ul-kontron-n6310-s-43.dts    | 120 +++++
 .../boot/dts/imx6ul-kontron-n6310-s-50.dts    | 120 +++++
 arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts  | 424 ++++++++++++++++++
 .../boot/dts/imx6ul-kontron-n6310-som.dtsi    | 137 ++++++
 6 files changed, 808 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts
 create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts
 create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts
 create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 7294ac36f4c0..afb61a55e26f 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -161,6 +161,10 @@ properties:
         items:
           - enum:
               - fsl,imx6ul-14x14-evk      # i.MX6 UltraLite 14x14 EVK Board
+              - kontron,n6310-som         # Kontron N6310 SOM
+              - kontron,n6310-s           # Kontron N6310 S Board
+              - kontron,n6310-s-43        # Kontron N6310 S 43 Board
+              - kontron,n6310-s-50        # Kontron N6310 S 50 Board
           - const: fsl,imx6ul
 
       - description: i.MX6ULL based Boards
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 9159fa2cea90..28b6cb3454a3 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -569,6 +569,9 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
 	imx6ul-geam.dtb \
 	imx6ul-isiot-emmc.dtb \
 	imx6ul-isiot-nand.dtb \
+	imx6ul-kontron-n6310-s.dtb \
+	imx6ul-kontron-n6310-s-43.dtb \
+	imx6ul-kontron-n6310-s-50.dtb \
 	imx6ul-liteboard.dtb \
 	imx6ul-opos6uldev.dtb \
 	imx6ul-pico-hobbit.dtb \
diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts
new file mode 100644
index 000000000000..63a0d8408fea
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 exceet electronics GmbH
+ * Copyright (C) 2018 Kontron Electronics GmbH
+ * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
+ */
+
+#include "imx6ul-kontron-n6310-s.dts"
+
+/ {
+	model = "Kontron N6310 S 43";
+	compatible = "kontron,n6310-s-43", "kontron,n6310-s",
+		     "kontron,n6310-som", "fsl,imx6ul";
+
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm7 0 5000000>;
+		brightness-levels = <0 4 8 16 32 64 128 255>;
+		default-brightness-level = <6>;
+		status = "okay";
+	};
+
+	panel {
+		compatible = "admatec,t043c004800272t2a";
+		backlight = <&backlight>;
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&display_out>;
+			};
+		};
+	};
+};
+
+&i2c4 {
+	gt911@5d {
+		compatible = "goodix,gt928";
+		reg = <0x5d>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_cap_touch>;
+		interrupt-parent = <&gpio5>;
+		interrupts = <6 8>;
+		reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
+		irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+&iomuxc {
+	pinctrl_lcdif_dat: lcdifdatgrp {
+		fsl,pins = <
+			MX6UL_PAD_LCD_DATA00__LCDIF_DATA00	0x79
+			MX6UL_PAD_LCD_DATA01__LCDIF_DATA01	0x79
+			MX6UL_PAD_LCD_DATA02__LCDIF_DATA02	0x79
+			MX6UL_PAD_LCD_DATA03__LCDIF_DATA03	0x79
+			MX6UL_PAD_LCD_DATA04__LCDIF_DATA04	0x79
+			MX6UL_PAD_LCD_DATA05__LCDIF_DATA05	0x79
+			MX6UL_PAD_LCD_DATA06__LCDIF_DATA06	0x79
+			MX6UL_PAD_LCD_DATA07__LCDIF_DATA07	0x79
+			MX6UL_PAD_LCD_DATA08__LCDIF_DATA08	0x79
+			MX6UL_PAD_LCD_DATA09__LCDIF_DATA09	0x79
+			MX6UL_PAD_LCD_DATA10__LCDIF_DATA10	0x79
+			MX6UL_PAD_LCD_DATA11__LCDIF_DATA11	0x79
+			MX6UL_PAD_LCD_DATA12__LCDIF_DATA12	0x79
+			MX6UL_PAD_LCD_DATA13__LCDIF_DATA13	0x79
+			MX6UL_PAD_LCD_DATA14__LCDIF_DATA14	0x79
+			MX6UL_PAD_LCD_DATA15__LCDIF_DATA15	0x79
+			MX6UL_PAD_LCD_DATA16__LCDIF_DATA16	0x79
+			MX6UL_PAD_LCD_DATA17__LCDIF_DATA17	0x79
+			MX6UL_PAD_LCD_DATA18__LCDIF_DATA18	0x79
+			MX6UL_PAD_LCD_DATA19__LCDIF_DATA19	0x79
+			MX6UL_PAD_LCD_DATA20__LCDIF_DATA20	0x79
+			MX6UL_PAD_LCD_DATA21__LCDIF_DATA21	0x79
+			MX6UL_PAD_LCD_DATA22__LCDIF_DATA22	0x79
+			MX6UL_PAD_LCD_DATA23__LCDIF_DATA23	0x79
+		>;
+	};
+
+	pinctrl_lcdif_ctrl: lcdifctrlgrp {
+		fsl,pins = <
+			MX6UL_PAD_LCD_CLK__LCDIF_CLK		0x79
+			MX6UL_PAD_LCD_ENABLE__LCDIF_ENABLE	0x79
+			MX6UL_PAD_LCD_HSYNC__LCDIF_HSYNC	0x79
+			MX6UL_PAD_LCD_VSYNC__LCDIF_VSYNC	0x79
+			MX6UL_PAD_LCD_RESET__LCDIF_RESET	0x79
+		>;
+	};
+
+	pinctrl_cap_touch: captouchgrp {
+		fsl,pins = <
+			MX6UL_PAD_SNVS_TAMPER6__GPIO5_IO06	0x1b0b0 /* Touch Interrupt */
+			MX6UL_PAD_SNVS_TAMPER7__GPIO5_IO07	0x1b0b0 /* Touch Reset */
+			MX6UL_PAD_SNVS_TAMPER8__GPIO5_IO08	0x1b0b0 /* Touch Wake */
+		>;
+	};
+
+	pinctrl_pwm7: pwm7grp {
+		fsl,pins = <
+			MX6UL_PAD_CSI_VSYNC__PWM7_OUT		0x110b0
+		>;
+	};
+};
+
+&lcdif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_lcdif_dat
+		     &pinctrl_lcdif_ctrl>;
+	status = "okay";
+
+	port {
+		display_out: endpoint {
+			remote-endpoint = <&panel_in>;
+		};
+	};
+};
+
+&pwm7 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pwm7>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts
new file mode 100644
index 000000000000..c3b133b3351e
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 exceet electronics GmbH
+ * Copyright (C) 2018 Kontron Electronics GmbH
+ * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
+ */
+
+#include "imx6ul-kontron-n6310-s.dts"
+
+/ {
+	model = "Kontron N6310 S 50";
+	compatible = "kontron,n6310-s-50", "kontron,n6310-s",
+		     "kontron,n6310-som", "fsl,imx6ul";
+
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm7 0 5000000>;
+		brightness-levels = <0 4 8 16 32 64 128 255>;
+		default-brightness-level = <6>;
+		status = "okay";
+	};
+
+	panel {
+		compatible = "admatec,t070p133t0s301";
+		backlight = <&backlight>;
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&display_out>;
+			};
+		};
+	};
+};
+
+&i2c4 {
+	gt911@5d {
+		compatible = "goodix,gt928";
+		reg = <0x5d>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_cap_touch>;
+		interrupt-parent = <&gpio5>;
+		interrupts = <6 8>;
+		reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
+		irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+&iomuxc {
+	pinctrl_lcdif_dat: lcdifdatgrp {
+		fsl,pins = <
+			MX6UL_PAD_LCD_DATA00__LCDIF_DATA00	0x79
+			MX6UL_PAD_LCD_DATA01__LCDIF_DATA01	0x79
+			MX6UL_PAD_LCD_DATA02__LCDIF_DATA02	0x79
+			MX6UL_PAD_LCD_DATA03__LCDIF_DATA03	0x79
+			MX6UL_PAD_LCD_DATA04__LCDIF_DATA04	0x79
+			MX6UL_PAD_LCD_DATA05__LCDIF_DATA05	0x79
+			MX6UL_PAD_LCD_DATA06__LCDIF_DATA06	0x79
+			MX6UL_PAD_LCD_DATA07__LCDIF_DATA07	0x79
+			MX6UL_PAD_LCD_DATA08__LCDIF_DATA08	0x79
+			MX6UL_PAD_LCD_DATA09__LCDIF_DATA09	0x79
+			MX6UL_PAD_LCD_DATA10__LCDIF_DATA10	0x79
+			MX6UL_PAD_LCD_DATA11__LCDIF_DATA11	0x79
+			MX6UL_PAD_LCD_DATA12__LCDIF_DATA12	0x79
+			MX6UL_PAD_LCD_DATA13__LCDIF_DATA13	0x79
+			MX6UL_PAD_LCD_DATA14__LCDIF_DATA14	0x79
+			MX6UL_PAD_LCD_DATA15__LCDIF_DATA15	0x79
+			MX6UL_PAD_LCD_DATA16__LCDIF_DATA16	0x79
+			MX6UL_PAD_LCD_DATA17__LCDIF_DATA17	0x79
+			MX6UL_PAD_LCD_DATA18__LCDIF_DATA18	0x79
+			MX6UL_PAD_LCD_DATA19__LCDIF_DATA19	0x79
+			MX6UL_PAD_LCD_DATA20__LCDIF_DATA20	0x79
+			MX6UL_PAD_LCD_DATA21__LCDIF_DATA21	0x79
+			MX6UL_PAD_LCD_DATA22__LCDIF_DATA22	0x79
+			MX6UL_PAD_LCD_DATA23__LCDIF_DATA23	0x79
+		>;
+	};
+
+	pinctrl_lcdif_ctrl: lcdifctrlgrp {
+		fsl,pins = <
+			MX6UL_PAD_LCD_CLK__LCDIF_CLK		0x79
+			MX6UL_PAD_LCD_ENABLE__LCDIF_ENABLE	0x79
+			MX6UL_PAD_LCD_HSYNC__LCDIF_HSYNC	0x79
+			MX6UL_PAD_LCD_VSYNC__LCDIF_VSYNC	0x79
+			MX6UL_PAD_LCD_RESET__LCDIF_RESET	0x79
+		>;
+	};
+
+	pinctrl_cap_touch: captouchgrp {
+		fsl,pins = <
+			MX6UL_PAD_SNVS_TAMPER6__GPIO5_IO06	0x1b0b0 /* Touch Interrupt */
+			MX6UL_PAD_SNVS_TAMPER7__GPIO5_IO07	0x1b0b0 /* Touch Reset */
+			MX6UL_PAD_SNVS_TAMPER8__GPIO5_IO08	0x1b0b0 /* Touch Wake */
+		>;
+	};
+
+	pinctrl_pwm7: pwm7grp {
+		fsl,pins = <
+			MX6UL_PAD_CSI_VSYNC__PWM7_OUT		0x110b0
+		>;
+	};
+};
+
+&lcdif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_lcdif_dat
+		     &pinctrl_lcdif_ctrl>;
+	status = "okay";
+
+	port {
+		display_out: endpoint {
+			remote-endpoint = <&panel_in>;
+		};
+	};
+};
+
+&pwm7 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pwm7>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts b/arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts
new file mode 100644
index 000000000000..5621a3dbf2dc
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts
@@ -0,0 +1,424 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 exceet electronics GmbH
+ * Copyright (C) 2018 Kontron Electronics GmbH
+ * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
+ */
+
+/dts-v1/;
+
+#include "imx6ul-kontron-n6310-som.dtsi"
+
+/ {
+	model = "Kontron N6310 S";
+	compatible = "kontron,n6310-s", "kontron,n6310-som", "fsl,imx6ul";
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm8 0 5000>;
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_gpio_leds>;
+
+		led1 {
+			label = "debug-led1";
+			gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+			linux,default-trigger = "heartbeat";
+		};
+
+		led2 {
+			label = "debug-led2";
+			gpios = <&gpio5 3 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		led3 {
+			label = "debug-led3";
+			gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+	};
+
+	regulators {
+		reg_3v3: regulator1 {
+			compatible = "regulator-fixed";
+			regulator-name = "3v3";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
+
+		reg_vref_adc: regulator2 {
+			compatible = "regulator-fixed";
+			regulator-name = "vref-adc";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
+
+		reg_usb_otg1_vbus: regulator3 {
+			compatible = "regulator-fixed";
+			regulator-name = "usb_otg1_vbus";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+		};
+	};
+
+};
+
+&adc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_adc1>;
+	num-channels = <3>;
+	vref-supply = <&reg_vref_adc>;
+	status = "okay";
+};
+
+&can2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_flexcan2>;
+	status = "okay";
+};
+
+&ecspi1 {
+	fsl,spi-num-chipselects = <1>;
+	cs-gpios = <&gpio4 26 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ecspi1>;
+	status = "okay";
+
+	fram@0 {
+		compatible = "atmel,at25";
+		reg = <0>;
+		spi-max-frequency = <20000000>;
+		spi-cpha;
+		spi-cpol;
+		pagesize = <1>;
+		size = <8192>;
+		address-width = <16>;
+	};
+};
+
+&fec1 {
+	pinctrl-0 = <&pinctrl_enet1>;
+	/delete-node/ mdio;
+};
+
+&fec2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_enet2 &pinctrl_enet2_mdio>;
+	phy-mode = "rmii";
+	phy-handle = <&ethphy2>;
+	status = "okay";
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy1: ethernet-phy@1 {
+			reg = <1>;
+			micrel,led-mode = <0>;
+			clocks = <&clks IMX6UL_CLK_ENET_REF>;
+			clock-names = "rmii-ref";
+		};
+
+		ethphy2: ethernet-phy@2 {
+			reg = <2>;
+			micrel,led-mode = <0>;
+			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
+			clock-names = "rmii-ref";
+		};
+	};
+};
+
+&i2c1 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	status = "okay";
+};
+
+&i2c4 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c4>;
+	status = "okay";
+
+	rtc@32 {
+		compatible = "epson,rx8900";
+		reg = <0x32>;
+	};
+};
+
+&iomuxc {
+	pinctrl-0 = <&pinctrl_reset_out &pinctrl_gpio>;
+
+	pinctrl_wdog: wdoggrp {
+		fsl,pins = <
+			MX6UL_PAD_GPIO1_IO09__WDOG1_WDOG_ANY	0x30b0
+		>;
+	};
+
+	pinctrl_gpio: gpio {
+		fsl,pins = <
+			MX6UL_PAD_SNVS_TAMPER5__GPIO5_IO05	0x1b0b0 /* DOUT1 */
+			MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04	0x1b0b0 /* DIN1 */
+			MX6UL_PAD_SNVS_TAMPER1__GPIO5_IO01	0x1b0b0 /* DOUT2 */
+			MX6UL_PAD_SNVS_TAMPER0__GPIO5_IO00	0x1b0b0 /* DIN2 */
+		>;
+	};
+
+	pinctrl_usbotg1: usbotg1 {
+		fsl,pins = <
+			MX6UL_PAD_GPIO1_IO04__GPIO1_IO04	0x1b0b0
+		>;
+	};
+
+	pinctrl_gpio_leds: gpio_leds {
+		fsl,pins = <
+			MX6UL_PAD_UART5_TX_DATA__GPIO1_IO30	0x1b0b0	/* LED H14 */
+			MX6UL_PAD_SNVS_TAMPER3__GPIO5_IO03	0x1b0b0	/* LED H15 */
+			MX6UL_PAD_SNVS_TAMPER2__GPIO5_IO02	0x1b0b0	/* LED H16 */
+		>;
+	};
+
+	/* FRAM */
+	pinctrl_ecspi1: ecspi1grp-1 {
+		fsl,pins = <
+			MX6UL_PAD_CSI_DATA07__ECSPI1_MISO	0x100b1
+			MX6UL_PAD_CSI_DATA06__ECSPI1_MOSI	0x100b1
+			MX6UL_PAD_CSI_DATA04__ECSPI1_SCLK	0x100b1
+			MX6UL_PAD_CSI_DATA05__GPIO4_IO26	0x100b1	/* ECSPI1-CS1 */
+		>;
+	};
+
+	pinctrl_enet2: enet2grp {
+		fsl,pins = <
+			MX6UL_PAD_ENET2_RX_EN__ENET2_RX_EN	0x1b0b0
+			MX6UL_PAD_ENET2_RX_ER__ENET2_RX_ER	0x1b0b0
+			MX6UL_PAD_ENET2_RX_DATA0__ENET2_RDATA00	0x1b0b0
+			MX6UL_PAD_ENET2_RX_DATA1__ENET2_RDATA01	0x1b0b0
+			MX6UL_PAD_ENET2_TX_EN__ENET2_TX_EN	0x1b0b0
+			MX6UL_PAD_ENET2_TX_DATA0__ENET2_TDATA00	0x1b0b0
+			MX6UL_PAD_ENET2_TX_DATA1__ENET2_TDATA01	0x1b0b0
+			MX6UL_PAD_ENET2_TX_CLK__ENET2_REF_CLK2	0x4001b009
+		>;
+	};
+
+	pinctrl_enet2_mdio: enet2mdiogrp {
+		fsl,pins = <
+			MX6UL_PAD_GPIO1_IO07__ENET2_MDC         0x1b0b0
+			MX6UL_PAD_GPIO1_IO06__ENET2_MDIO        0x1b0b0
+		>;
+	};
+
+	pinctrl_flexcan2: flexcan2grp{
+		fsl,pins = <
+			MX6UL_PAD_UART2_RTS_B__FLEXCAN2_RX	0x1b020
+			MX6UL_PAD_UART2_CTS_B__FLEXCAN2_TX	0x1b020
+		>;
+	};
+
+	pinctrl_pwm8: pwm8grp {
+		fsl,pins = <
+			MX6UL_PAD_CSI_HSYNC__PWM8_OUT		0x110b0
+		>;
+	};
+
+	pinctrl_uart1: uart1grp {
+		fsl,pins = <
+			MX6UL_PAD_UART1_TX_DATA__UART1_DCE_TX	0x1b0b1
+			MX6UL_PAD_UART1_RX_DATA__UART1_DCE_RX	0x1b0b1
+		>;
+	};
+
+	pinctrl_uart2: uart2grp {
+		fsl,pins = <
+			MX6UL_PAD_NAND_DATA04__UART2_DCE_TX	0x1b0b1
+			MX6UL_PAD_NAND_DATA05__UART2_DCE_RX	0x1b0b1
+			MX6UL_PAD_NAND_DATA06__UART2_DCE_CTS	0x1b0b1
+			/*
+			 * mux unused RTS to make sure it doesn't cause
+			 * any interrupts when it is undefined
+			 */
+			MX6UL_PAD_NAND_DATA07__UART2_DCE_RTS	0x1b0b1
+		>;
+	};
+
+	pinctrl_uart3: uart3grp {
+		fsl,pins = <
+			MX6UL_PAD_UART3_TX_DATA__UART3_DCE_TX	0x1b0b1
+			MX6UL_PAD_UART3_RX_DATA__UART3_DCE_RX	0x1b0b1
+			MX6UL_PAD_UART3_CTS_B__UART3_DCE_CTS	0x1b0b1
+			MX6UL_PAD_UART3_RTS_B__UART3_DCE_RTS	0x1b0b1
+		>;
+	};
+
+	pinctrl_uart4: uart4grp {
+		fsl,pins = <
+			MX6UL_PAD_UART4_TX_DATA__UART4_DCE_TX	0x1b0b1
+			MX6UL_PAD_UART4_RX_DATA__UART4_DCE_RX	0x1b0b1
+		>;
+	};
+
+	pinctrl_usdhc1: usdhc1grp {
+		fsl,pins = <
+			MX6UL_PAD_SD1_CMD__USDHC1_CMD		0x17059
+			MX6UL_PAD_SD1_CLK__USDHC1_CLK		0x10059
+			MX6UL_PAD_SD1_DATA0__USDHC1_DATA0	0x17059
+			MX6UL_PAD_SD1_DATA1__USDHC1_DATA1	0x17059
+			MX6UL_PAD_SD1_DATA2__USDHC1_DATA2	0x17059
+			MX6UL_PAD_SD1_DATA3__USDHC1_DATA3	0x17059
+			MX6UL_PAD_UART1_RTS_B__GPIO1_IO19	0x100b1	/* SD1_CD */
+		>;
+	};
+
+	pinctrl_usdhc2: usdhc2grp {
+		fsl,pins = <
+			MX6UL_PAD_NAND_RE_B__USDHC2_CLK		0x10059
+			MX6UL_PAD_NAND_WE_B__USDHC2_CMD		0x17059
+			MX6UL_PAD_NAND_DATA00__USDHC2_DATA0	0x17059
+			MX6UL_PAD_NAND_DATA01__USDHC2_DATA1	0x17059
+			MX6UL_PAD_NAND_DATA02__USDHC2_DATA2	0x17059
+			MX6UL_PAD_NAND_DATA03__USDHC2_DATA3	0x17059
+		>;
+	};
+
+	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
+		fsl,pins = <
+			MX6UL_PAD_NAND_RE_B__USDHC2_CLK		0x100b9
+			MX6UL_PAD_NAND_WE_B__USDHC2_CMD		0x170b9
+			MX6UL_PAD_NAND_DATA00__USDHC2_DATA0	0x170b9
+			MX6UL_PAD_NAND_DATA01__USDHC2_DATA1	0x170b9
+			MX6UL_PAD_NAND_DATA02__USDHC2_DATA2	0x170b9
+			MX6UL_PAD_NAND_DATA03__USDHC2_DATA3	0x170b9
+		>;
+	};
+
+	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
+		fsl,pins = <
+			MX6UL_PAD_NAND_RE_B__USDHC2_CLK		0x100f9
+			MX6UL_PAD_NAND_WE_B__USDHC2_CMD		0x170f9
+			MX6UL_PAD_NAND_DATA00__USDHC2_DATA0	0x170f9
+			MX6UL_PAD_NAND_DATA01__USDHC2_DATA1	0x170f9
+			MX6UL_PAD_NAND_DATA02__USDHC2_DATA2	0x170f9
+			MX6UL_PAD_NAND_DATA03__USDHC2_DATA3	0x170f9
+		>;
+	};
+
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			MX6UL_PAD_CSI_PIXCLK__I2C1_SCL		0x4001b8b0
+			MX6UL_PAD_CSI_MCLK__I2C1_SDA		0x4001b8b0
+		>;
+	};
+
+	pinctrl_i2c4: i2c4grp {
+		fsl,pins = <
+			MX6UL_PAD_UART2_TX_DATA__I2C4_SCL	0x4001f8b0
+			MX6UL_PAD_UART2_RX_DATA__I2C4_SDA	0x4001f8b0
+		>;
+	};
+
+	pinctrl_adc1: adc1grp {
+		fsl,pins = <
+			MX6UL_PAD_GPIO1_IO02__GPIO1_IO02	0xb0
+			MX6UL_PAD_GPIO1_IO03__GPIO1_IO03	0xb0
+			MX6UL_PAD_GPIO1_IO08__GPIO1_IO08	0xb0
+		>;
+	};
+};
+
+&pwm8 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pwm8>;
+	status = "okay";
+};
+
+&snvs_poweroff {
+	status = "okay";
+};
+
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart1>;
+	status = "okay";
+};
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart2>;
+	linux,rs485-enabled-at-boot-time;
+	rs485-rx-during-tx;
+	rs485-rts-active-low;
+	uart-has-rtscts;
+	status = "okay";
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart3>;
+	fsl,uart-has-rtscts;
+	status = "okay";
+};
+
+&uart4 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart4>;
+	status = "okay";
+};
+
+&usbotg1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usbotg1>;
+	dr_mode = "otg";
+	status = "okay";
+	srp-disable;
+	hnp-disable;
+	adp-disable;
+	vbus-supply = <&reg_usb_otg1_vbus>;
+};
+
+&usbotg2 {
+	dr_mode = "host";
+	disable-over-current;
+	status = "okay";
+};
+
+&usdhc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usdhc1>;
+	cd-gpios = <&gpio1 19 GPIO_ACTIVE_LOW>;
+	keep-power-in-suspend;
+	enable-sdio-wakeup;
+	vmmc-supply = <&reg_3v3>;
+	voltage-ranges = <3300 3300>;
+	no-1-8-v;
+	status = "okay";
+};
+
+&usdhc2 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc2>;
+	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
+	tuning-step = <2>;
+	non-removable;
+	keep-power-in-suspend;
+	enable-sdio-wakeup;
+	vmmc-supply = <&reg_3v3>;
+	voltage-ranges = <3300 3300>;
+	no-1-8-v;
+	status = "okay";
+};
+
+&wdog1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_wdog>;
+	status = "okay";
+	fsl,ext-reset-output;
+};
diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi
new file mode 100644
index 000000000000..084a8ccd574e
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 exceet electronics GmbH
+ * Copyright (C) 2018 Kontron Electronics GmbH
+ * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
+ */
+
+#include "imx6ul.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	model = "Kontron N6310 SOM";
+	compatible = "kontron,n6310-som", "fsl,imx6ul";
+
+	memory@80000000 {
+		reg = <0x80000000 0x10000000>;
+	};
+};
+
+&cpu0 {
+	clock-frequency = <528000000>;
+};
+
+&ecspi2 {
+	cs-gpios = <&gpio4 22 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ecspi2>;
+	status = "okay";
+
+	flash: mx25v80@0 {
+		compatible = "mxicy,mx25v8035f", "jedec,spi-nor";
+		spi-max-frequency = <50000000>;
+		reg = <0>;
+	};
+};
+
+&fec1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_enet1 &pinctrl_enet1_mdio>;
+	phy-mode = "rmii";
+	phy-handle = <&ethphy1>;
+	status = "okay";
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy1: ethernet-phy@1 {
+			reg = <1>;
+			micrel,led-mode = <0>;
+			clocks = <&clks IMX6UL_CLK_ENET_REF>;
+			clock-names = "rmii-ref";
+		};
+	};
+};
+
+&fec2 {
+	phy-mode = "rmii";
+	status = "disabled";
+};
+
+&iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_reset_out>;
+
+	pinctrl_reset_out: rstoutgrp {
+		fsl,pins = <
+			MX6UL_PAD_SNVS_TAMPER9__GPIO5_IO09      0x1b0b0
+		>;
+	};
+
+	pinctrl_ecspi2: ecspi2grp {
+		fsl,pins = <
+			MX6UL_PAD_CSI_DATA03__ECSPI2_MISO      0x100b1
+			MX6UL_PAD_CSI_DATA02__ECSPI2_MOSI      0x100b1
+			MX6UL_PAD_CSI_DATA00__ECSPI2_SCLK      0x100b1
+			MX6UL_PAD_CSI_DATA01__GPIO4_IO22       0x100b1
+		>;
+	};
+
+	pinctrl_enet1: enet1grp {
+		fsl,pins = <
+			MX6UL_PAD_ENET1_RX_EN__ENET1_RX_EN      0x1b0b0
+			MX6UL_PAD_ENET1_RX_ER__ENET1_RX_ER      0x1b0b0
+			MX6UL_PAD_ENET1_RX_DATA0__ENET1_RDATA00 0x1b0b0
+			MX6UL_PAD_ENET1_RX_DATA1__ENET1_RDATA01 0x1b0b0
+			MX6UL_PAD_ENET1_TX_EN__ENET1_TX_EN      0x1b0b0
+			MX6UL_PAD_ENET1_TX_DATA0__ENET1_TDATA00 0x1b0b0
+			MX6UL_PAD_ENET1_TX_DATA1__ENET1_TDATA01 0x1b0b0
+			MX6UL_PAD_ENET1_TX_CLK__ENET1_REF_CLK1  0x4001b009
+		>;
+	};
+
+	pinctrl_enet1_mdio: enet1mdiogrp {
+		fsl,pins = <
+			MX6UL_PAD_GPIO1_IO07__ENET1_MDC         0x1b0b0
+			MX6UL_PAD_GPIO1_IO06__ENET1_MDIO        0x1b0b0
+		>;
+	};
+
+	pinctrl_qspi: qspigrp {
+		fsl,pins = <
+			MX6UL_PAD_NAND_WP_B__QSPI_A_SCLK        0x70a1
+			MX6UL_PAD_NAND_READY_B__QSPI_A_DATA00   0x70a1
+			MX6UL_PAD_NAND_CE0_B__QSPI_A_DATA01     0x70a1
+			MX6UL_PAD_NAND_CE1_B__QSPI_A_DATA02     0x70a1
+			MX6UL_PAD_NAND_CLE__QSPI_A_DATA03       0x70a1
+			MX6UL_PAD_NAND_DQS__QSPI_A_SS0_B        0x70a1
+		>;
+	};
+};
+
+&qspi {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_qspi>;
+	status = "okay";
+
+	flash0: w25m02gv@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "spi-nand";
+		spi-max-frequency = <108000000>;
+		spi-tx-bus-width = <4>;
+		spi-rx-bus-width = <4>;
+		reg = <0>;
+
+		partition@0 {
+			label = "ubi1";
+			reg = <0x00000000 0x08000000>;
+		};
+
+		partition@8000000 {
+			label = "ubi2";
+			reg = <0x08000000 0x08000000>;
+		};
+	};
+};
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 1/2] dt-bindings: vendor-prefixes: Add Admatec AG
From: Krzysztof Kozlowski @ 2019-07-26  6:17 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-kernel, linux-arm-kernel, Schrempf Frieder
  Cc: Krzysztof Kozlowski

Add vendor prefix for Admatec AG.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Changes since v1:
New patch
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6992bbbbffab..94c816f74209 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -43,6 +43,8 @@ patternProperties:
     description: AD Holdings Plc.
   "^adi,.*":
     description: Analog Devices, Inc.
+  "^admatec,.*":
+    description: Admatec AG
   "^advantech,.*":
     description: Advantech Corporation
   "^aeroflexgaisler,.*":
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v2 1/2] dt-bindings: panel: Add parallel RGB mode for Ilitek ILI9341 panels
From: Josef Luštický @ 2019-07-26  5:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sam Ravnborg, dri-devel, devicetree, Daniel Vetter,
	thierry.reding, airlied
In-Reply-To: <20190724195744.GA22947@bogus>

[-- Attachment #1: Type: text/plain, Size: 4786 bytes --]

Hello Rob,
my fault, sorry, backlight should not be required.
I'll fix this in the next patch version.

I'd like to post the DT bindings in the YAML format.
Work-in-progress YAML bindings file is in the attachment,
any comments would be highly appreciated.

Kind regards
Josef

st 24. 7. 2019 v 21:57 odesílatel Rob Herring <robh@kernel.org> napsal:
>
> On Mon, Jul 08, 2019 at 04:56:17PM +0200, Josef Lusticky wrote:
> > ILI9341 supports both SPI input mode and parallel RGB input mode.
> > This commit adds parallel RGB input mode bindings.
> >
> > Signed-off-by: Josef Lusticky <josef@lusticky.cz>
> > ---
> >  .../bindings/display/ilitek,ili9341.txt       | 67 ++++++++++++++++---
> >  1 file changed, 56 insertions(+), 11 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> > index 169b32e4ee4e..629f38a1d0cd 100644
> > --- a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> > +++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> > @@ -1,27 +1,72 @@
> >  Ilitek ILI9341 display panels
> >
> > -This binding is for display panels using an Ilitek ILI9341 controller in SPI
> > -mode.
> > +This binding is for display panels using an Ilitek ILI9341 controller.
> > +The display panels are supported in the following graphical input modes:
> > +- SPI input mode
> > +     MIPI-DBI Type 3 Option 1 or Option 3 is used to transfer
> > +     commands and graphical data
> > +- parallel RGB input mode
> > +     MIPI-DBI Type 3 Option 1 or Option 3 is used for commands
> > +     MIPI-DPI 18-bit parallel RGB connection is used to transfer
> > +     graphical data
> >
> > -Required properties:
> > -- compatible:        "adafruit,yx240qv29", "ilitek,ili9341"
> > -- dc-gpios:  D/C pin
> > -- reset-gpios:       Reset pin
> > +
> > +SPI input mode:
> >
> >  The node for this driver must be a child node of a SPI controller, hence
> > -all mandatory properties described in ../spi/spi-bus.txt must be specified.
> > +all mandatory properties described in spi/spi-bus.txt must be specified.
> > +
> > +Required properties in SPI input mode:
> > +- compatible:   "adafruit,yx240qv29", "ilitek,ili9341"
> > +- backlight:    phandle of the backlight device attached to the panel
>
> Why is backlight now required?
>
> > +
> > +Optional properties in SPI input mode:
> > +- rotation:     panel rotation in degrees counter clockwise (0,90,180,270)
> > +- dc-gpios:     GPIO spec for the D/C pin, see gpio/gpio.txt
> > +- reset-gpios:  GPIO spec for the reset pin, see gpio/gpio.txt
> > +
> > +
> > +Parallel RGB input mode:
> > +
> > +The node for this driver must be a child node of a SPI controller, hence
> > +all mandatory properties described in spi/spi-bus.txt must be specified.
> > +
> > +Required properties in parallel RGB input mode:
> > +- compatible:   "displaytech,dt024ctft", "ilitek,ili9341"
> > +- backlight:    phandle of the backlight device attached to the panel
> > +
> > +Optional properties in parallel RGB input mode:
> > +- dc-gpios:     GPIO spec for the D/C pin, see gpio/gpio.txt
> > +- reset-gpios:  GPIO spec for the reset pin, see gpio/gpio.txt
> >
> > -Optional properties:
> > -- rotation:  panel rotation in degrees counter clockwise (0,90,180,270)
> > -- backlight: phandle of the backlight device attached to the panel
> > +In parallel RGB input mode,
> > +the device node can contain one 'port' child node with one child
> > +'endpoint' node, according to the bindings defined in
> > +media/video-interfaces.txt. This node should describe panel's video bus.
> >
> > -Example:
> > +
> > +Example in SPI input mode:
> >       display@0{
> >               compatible = "adafruit,yx240qv29", "ilitek,ili9341";
> >               reg = <0>;
> >               spi-max-frequency = <32000000>;
> >               dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> >               reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> > +             backlight = <&backlight>;
> >               rotation = <270>;
> > +     };
> > +
> > +Example in parallel RGB input mode:
> > +     panel@{
> > +             compatible = "displaytech,dt024ctft", "ilitek,ili9341";
> > +             reg = <0>;
> > +             spi-max-frequency = <32000000>;
> > +             dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> > +             reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> >               backlight = <&backlight>;
> > +             port {
> > +                     panel_in: endpoint {
> > +                             remote-endpoint = <&display_out>;
> > +                     };
> > +             };
> >       };
> > --
> > 2.20.1
> >

[-- Attachment #2: ilitek,ili9341.txt --]
[-- Type: text/plain, Size: 1622 bytes --]

# SPDX-License-Identifier: GPL-2.0
%YAML 1.2
---
$id: http://devicetree.org/schemas/display/panel/ilitek,ili9341.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

title: Ilitek ILI9341 display panels

maintainers:
  - Josef Lusticky <josef@lusticky.cz>

description: |
  This binding is for display panels using an Ilitek ILI9341 controller.
  The display panels are supported in the following graphical input modes:
  SPI input mode
    MIPI-DBI Type 3 Option 1 or Option 3 is used to transfer
    commands and graphical data
  Parallel RGB input mode
    MIPI-DBI Type 3 Option 1 or Option 3 is used for commands
    MIPI-DPI 18-bit parallel RGB connection is used to transfer
    graphical data

allOf:
  - $ref: /schemas/spi/spi-controller.yaml#

properties:
  compatible:
    items:
      - const: adafruit,yx240qv29

  backlight: true
  reset-gpios: true
  dc-gpios: true
  rotation: true
  port: true

additionalProperties: false

required:
  - compatible
  - backlight

Example in SPI input mode:
	display@0{
		compatible = "adafruit,yx240qv29", "ilitek,ili9341";
		reg = <0>;
		spi-max-frequency = <32000000>;
		dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
		backlight = <&backlight>;
		rotation = <270>;
	};

Example in parallel RGB input mode:
	panel@{
		compatible = "displaytech,dt024ctft", "ilitek,ili9341";
		reg = <0>;
		spi-max-frequency = <32000000>;
		dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
		backlight = <&backlight>;
		port {
			panel_in: endpoint {
				remote-endpoint = <&display_out>;
			};
		};
	};

^ permalink raw reply

* Re: [PATCH v2 2/2] mmc: Add support for the ASPEED SD controller
From: Adrian Hunter @ 2019-07-26  5:56 UTC (permalink / raw)
  To: Andrew Jeffery, linux-mmc
  Cc: Ulf Hansson, Rob Herring, mark.rutland, Joel Stanley, devicetree,
	linux-arm-kernel, linux-aspeed, linux-kernel, Ryan Chen
In-Reply-To: <7cd30f3d-43fd-42da-9301-091eb2625c65@www.fastmail.com>

On 26/07/19 3:52 AM, Andrew Jeffery wrote:
> On Thu, 25 Jul 2019, at 22:49, Adrian Hunter wrote:
>> On 12/07/19 6:32 AM, Andrew Jeffery wrote:
>>> +static int aspeed_sdhci_probe(struct platform_device *pdev)
>>> +{
>>> +	struct sdhci_pltfm_host *pltfm_host;
>>> +	struct aspeed_sdhci *dev;
>>> +	struct sdhci_host *host;
>>> +	struct resource *res;
>>> +	int slot;
>>> +	int ret;
>>> +
>>> +	host = sdhci_pltfm_init(pdev, &aspeed_sdc_pdata, sizeof(*dev));
>>> +	if (IS_ERR(host))
>>> +		return PTR_ERR(host);
>>> +
>>> +	pltfm_host = sdhci_priv(host);
>>> +	dev = sdhci_pltfm_priv(pltfm_host);
>>> +	dev->parent = dev_get_drvdata(pdev->dev.parent);
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	slot = aspeed_sdhci_calculate_slot(dev, res);
>>> +	if (slot < 0)
>>> +		return slot;
>>> +	dev_info(&pdev->dev, "Configuring for slot %d\n", slot);
>>> +	dev->width_mask = !slot ? ASPEED_SDC_S0MMC8 : ASPEED_SDC_S1MMC8;
>>
>> That implies that you only support 2 slots which begs the question why
>> you don't validate slot.
> 
> I'm not sure what you mean here, but I'll dig into it.

I just meant, if you only support 2 slots:

	if (slot > 1)
		return -EINVAL;

^ permalink raw reply

* Re: [RFC,v3 6/9] media: platform: Add Mediatek ISP P1 V4L2 functions
From: Tomasz Figa @ 2019-07-26  5:49 UTC (permalink / raw)
  To: Jungo Lin, Hans Verkuil
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	Sean Cheng (鄭昇弘),
	Frederic Chen (陳俊元),
	Rynn Wu (吳育恩), srv_heupstream, Rob Herring,
	Ryan Yu (余孟修),
	Frankie Chiu (邱文凱),
	ddavenport-F7+t8E8rja9g9hUCZPvPmw, Sj Huang,
	moderated list:ARM/Mediatek SoC support, Laurent Pinchart,
	Matthias Brugger, Mauro Carvalho Chehab,
	list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS <iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>, 
In-Reply-To: <1563942689.1212.494.camel@mtksdccf07>

On Wed, Jul 24, 2019 at 1:31 PM Jungo Lin <jungo.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>
> Hi, Tomasz:
>
> On Tue, 2019-07-23 at 19:21 +0900, Tomasz Figa wrote:
> > Hi Jungo,
> >
> > On Thu, Jul 18, 2019 at 1:39 PM Jungo Lin <jungo.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > >
> > > Hi, Tomasz:
> > >
> > > On Wed, 2019-07-10 at 18:54 +0900, Tomasz Figa wrote:
> > > > Hi Jungo,
> > > >
> > > > On Tue, Jun 11, 2019 at 11:53:41AM +0800, Jungo Lin wrote:
> > [snip]
> > > > > +static void mtk_cam_req_try_isp_queue(struct mtk_cam_dev *cam_dev,
> > > > > +                                 struct media_request *new_req)
> > > > > +{
> > > > > +   struct mtk_cam_dev_request *req, *req_safe, *cam_dev_req;
> > > > > +   struct device *dev = &cam_dev->pdev->dev;
> > > > > +
> > > > > +   dev_dbg(dev, "%s new req:%d", __func__, !new_req);
> > > > > +
> > > > > +   if (!cam_dev->streaming) {
> > > > > +           cam_dev_req = mtk_cam_req_to_dev_req(new_req);
> > > > > +           spin_lock(&cam_dev->req_lock);
> > > > > +           list_add_tail(&cam_dev_req->list, &cam_dev->req_list);
> > > > > +           spin_unlock(&cam_dev->req_lock);
> > > > > +           dev_dbg(dev, "%s: stream off, no ISP enqueue\n", __func__);
> > > > > +           return;
> > > > > +   }
> > > > > +
> > > > > +   /* Normal enqueue flow */
> > > > > +   if (new_req) {
> > > > > +           mtk_isp_req_enqueue(dev, new_req);
> > > > > +           return;
> > > > > +   }
> > > > > +
> > > > > +   /* Flush all media requests wehen first stream on */
> > > > > +   list_for_each_entry_safe(req, req_safe, &cam_dev->req_list, list) {
> > > > > +           list_del(&req->list);
> > > > > +           mtk_isp_req_enqueue(dev, &req->req);
> > > > > +   }
> > > > > +}
> > > >
> > > > This will have to be redone, as per the other suggestions, but generally one
> > > > would have a function that tries to queue as much as possible from a list to
> > > > the hardware and another function that adds a request to the list and calls
> > > > the first function.
> > > >
> > >
> > > We revised this function as below.
> > > First to check the en-queue conditions:
> > > a. stream on
> > > b. The composer buffers in SCP are 3, so we only could has 3 jobs
> > > at the same time.
> > >
> > >
> > > Second, try to en-queue the frames in the pending job if possible and
> > > move them into running job list if possible.
> > >
> > > The request has been inserted into pending job in mtk_cam_req_validate
> > > which is used to validate media_request.
> >
> > Thanks for replying to each of the comments, that's very helpful.
> > Snipped out the parts that I agreed with.
> >
> > Please note that req_validate is not supposed to change any driver
> > state. It's only supposed to validate the request. req_queue is the
> > right callback to insert the request into some internal driver
> > bookkeeping structures.
> >
>
> Yes, in req_validate function, we don't change any driver state.
> Below is the function's implementation.
>
> a. Call vb2_request_validate(req) to verify media request.
> b. Update the buffer internal structure buffer.
> c. Insert the request into pending_job_list to prepare en-queue.
>

Adding to a list is changing driver state. The callback must not
modify anything else than the request itself.

Queuing to driver's list should happen in req_queue instead.

[snip]
> > >
> > > void mtk_cam_dev_req_try_queue(struct mtk_cam_dev *cam_dev)
> > > {
> > >         struct mtk_cam_dev_request *req, *req_prev;
> > >         struct list_head enqueue_job_list;
> > >         int buffer_cnt = atomic_read(&cam_dev->running_job_count);
> > >         unsigned long flags;
> > >
> > >         if (!cam_dev->streaming ||
> > >             buffer_cnt >= MTK_ISP_MAX_RUNNING_JOBS) {
> >
> > Do we have a guarantee that cam_dev->running_job_count doesn't
> > decrement between the atomic_read() above and this line?
> >
>
> Ok, we will use cam->pending_job_lock to protect
> cam_dev->running_job_count access. Below is the revised version.
>
> void mtk_cam_dev_req_try_queue(struct mtk_cam_dev *cam)
> {
>         struct mtk_cam_dev_request *req, *req_prev;
>         unsigned long flags;
>
>         if (!cam->streaming) {
>                 dev_dbg(cam->dev, "stream is off\n");
>                 return;
>         }
>
>         spin_lock_irqsave(&cam->pending_job_lock, flags);
>         if (atomic_read(&cam->running_job_count) >= MTK_ISP_MAX_RUNNING_JOBS) {

If we use a spin_lock to protect the counter, perhaps we don't need
the atomic type anymore?

>                 dev_dbg(cam->dev, "jobs are full\n");
>                 spin_unlock_irqrestore(&cam->pending_job_lock, flags);
>                 return;
>         }
>         list_for_each_entry_safe(req, req_prev, &cam->pending_job_list, list) {

Could we instead check the counter here and break if it's >=
MTK_ISP_MAX_RUNNING_JOBS?
Then we could increment it here too to simplify the code.

>                 list_del(&req->list);
>                 spin_lock_irqsave(&cam->running_job_lock, flags);
>                 list_add_tail(&req->list, &cam->running_job_list);
>                 mtk_isp_req_enqueue(cam, req);
>                 spin_unlock_irqrestore(&cam->running_job_lock, flags);
>                 if (atomic_inc_return(&cam->running_job_count) >=
>                         MTK_ISP_MAX_RUNNING_JOBS)
>                         break;

With the above suggestion, this if block would go away.

[snip]
> > >                 mtk_isp_req_enqueue(cam_dev, req);
> > >         }
> > > }
> > >
> > [snip]
> > > > > +   stride = DIV_ROUND_UP(stride * pixel_byte, 8);
> > > > > +
> > > > > +   if (pix_fmt == V4L2_PIX_FMT_MTISP_F10)
> > > > > +           stride = ALIGN(stride, 4);
> > > >
> > > > Is it expected that only the F10 format needs this alignment?
> > > >
> > >
> > > yes, if the pixel bits of image format is 10, the byte alignment of bpl
> > > should be 4. Otherwise, it is 8. We will revise this and add more
> > > comments.
> >
> > That means that the B10 format also needs the extra alignment, as
> > opposed to what the original code did, right?
> >
>
> Sorry for short code snippet.
> This alignment checking is only applied to F10, no B10.
> If you like to check the full function, you could check this in this
> link[1].
>
> static void cal_image_pix_mp(struct mtk_cam_dev *cam, unsigned int
> node_id,
>                              struct v4l2_pix_format_mplane *mp)
> {
>         unsigned int bpl, ppl;
>         unsigned int pixel_bits = get_pixel_bits(mp->pixelformat);
>         unsigned int width = mp->width;
>
>         if (node_id == MTK_CAM_P1_MAIN_STREAM_OUT) {
>                 /* bayer encoding format & 2 bytes alignment */
>                 bpl = ALIGN(DIV_ROUND_UP(width * pixel_bits, 8), 2);
>         } else if (node_id == MTK_CAM_P1_PACKED_BIN_OUT) {
>                 /*
>                  * The FULL-G encoding format
>                  * 1 G component per pixel
>                  * 1 R component per 4 pixel
>                  * 1 B component per 4 pixel
>                  * Total 4G/1R/1B in 4 pixel (pixel per line:ppl)
>                  */
>                 ppl = DIV_ROUND_UP(width * 6, 4);
>                 bpl = DIV_ROUND_UP(ppl * pixel_bits, 8);
>
>                 /* 4 bytes alignment for 10 bit & others are 8 bytes */
>                 if (pixel_bits == 10)
>                         bpl = ALIGN(bpl, 4);
>                 else
>                         bpl = ALIGN(bpl, 8);
>         }
>
> [1]
> https://crrev.com/c/1712885/2/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c#303
>

Got it, thanks!

[snip]
> > > > > +
> > > > > +static struct v4l2_subdev *
> > > > > +mtk_cam_cio_get_active_sensor(struct mtk_cam_dev *cam_dev)
> > > > > +{
> > > > > +   struct media_device *mdev = cam_dev->seninf->entity.graph_obj.mdev;
> > > > > +   struct media_entity *entity;
> > > > > +   struct device *dev = &cam_dev->pdev->dev;
> > > > > +   struct v4l2_subdev *sensor;
> > > >
> > > > This variable would be unitialized if there is no streaming sensor. Was
> > > > there no compiler warning generated for this?
> > > >
> > >
> > > No, there is no compiler warning.
> > > But, we will assign sensor to NULL to avoid unnecessary compiler warning
> > > with different compiler options.
> > >
> >
> > Thanks. It would be useful if you could check why the compiler you're
> > using doesn't show a warning here. We might be missing other
> > uninitialized variables.
> >
>
> We will feedback to your project team to check the possible reason about
> compiler warning issue.
>

Do you mean that it was the Clang toolchain used on Chromium OS (e.g.
emerge chromeos-kernel-4_19)?

[snip]
> > > > > +
> > > > > +   dev_dbg(dev, "%s: node:%d fd:%d idx:%d\n",
> > > > > +           __func__,
> > > > > +           node->id,
> > > > > +           buf->vbb.request_fd,
> > > > > +           buf->vbb.vb2_buf.index);
> > > > > +
> > > > > +   /* For request buffers en-queue, handled in mtk_cam_req_try_queue */
> > > > > +   if (vb->vb2_queue->uses_requests)
> > > > > +           return;
> > > >
> > > > I'd suggest removing non-request support from this driver. Even if we end up
> > > > with a need to provide compatibility for non-request mode, then it should be
> > > > built on top of the requests mode, so that the driver itself doesn't have to
> > > > deal with two modes.
> > > >
> > >
> > > The purpose of non-request function in this driver is needed by
> > > our camera middle-ware design. It needs 3A statistics buffers before
> > > image buffers en-queue. So we need to en-queue 3A statistics with
> > > non-request mode in this driver. After MW got the 3A statistics data, it
> > > will en-queue the images, tuning buffer and other meta buffers with
> > > request mode. Based on this requirement, do you have any suggestion?
> > > For upstream driver, should we only consider request mode?
> > >
> >
> > Where does that requirement come from? Why the timing of queuing of
> > the buffers to the driver is important?
> >
> > [snip]
>
> Basically, this requirement comes from our internal camera
> middle-ware/3A hal in user space. Since this is not generic requirement,
> we will follow your original suggestion to keep the request mode only
> and remove other non-request design in other files. For upstream driver,
> it should support request mode only.
>

Note that Chromium OS will use the "upstream driver" and we don't want
to diverge, so please make the userspace also use only requests. I
don't see a reason why there would be any need to submit any buffers
outside of a request.

[snip]
> > > > > +static void mtk_cam_vb2_buf_request_complete(struct vb2_buffer *vb)
> > > > > +{
> > > > > +   struct mtk_cam_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
> > > > > +
> > > > > +   v4l2_ctrl_request_complete(vb->req_obj.req,
> > > > > +                              dev->v4l2_dev.ctrl_handler);
> > > >
> > > > This would end up being called multiple times, once for each video node.
> > > > Instead, this should be called explicitly by the driver when it completed
> > > > the request - perhaps in the frame completion handler?
> > > >
> > > > With that, we probably wouldn't even need this callback.
> > > >
> > >
> > > First, if we don't implement this callback function, we will receive
> > > kernel warning as below.
> > >
> > > https://elixir.bootlin.com/linux/latest/source/drivers/media/common/videobuf2/videobuf2-v4l2.c#L420
> > >
> > > Second, this function is only be called in __vb2_queue_cancel function.
> > > Moreover, we will remove cam_dev->v4l2_dev.ctrl_handler in next patch.
> > > So could we just implement dummy empty function?
> > >
> > >  * @buf_request_complete: a buffer that was never queued to the driver
> > > but is
> > >  *                      associated with a queued request was canceled.
> > >  *                      The driver will have to mark associated objects in the
> > >  *                      request as completed; required if requests are
> > >  *                      supported.
> > >
> >
> > Good catch, thanks.
> >
> > Sounds like we may indeed need to implement this callback. In
> > particular, we may need to remove the request that the buffer was
> > associated with from the driver queue and return the other buffers
> > associated to it with an error state. This should be similar to
> > handling a request failure.
> > [snip]
>
> Before calling this callback function, the VB2's stop_streaming has been
> called. Normally, we will return the buffers belonged to this vb2 queu
> with error state. On other hand, only if the state of request is
> MEDIA_REQUEST_STATE_QUEUED, the buf_request_complete will be called in
> __vb2_queue_cancel function. It hints this media request has been
> validated and inserted into our driver's pending_job_list or
> running_job_list. So we will call mtk_cam_dev_req_cleanup() remove these
> requests from driver's list when streaming is off. Since we have no
> v4l2_ctrl, do we need to do the above things which is already handled in
> mtk_cam_vb2_stop_streaming function? Maybe is this callback function
> only designed for v4l2_ctrl_request_complete usage?

Are you sure that this callback can be only called after
stop_streaming? Also wouldn't that be after stop_streaming only on 1
queue? The other queues could still remain streaming, but we still
have to return corresponding buffers I believe.

Hans, could you clarify what exactly this callback is supposed to do?

>
> static void mtk_cam_dev_req_cleanup(struct mtk_cam_dev *cam)
> {
>         struct mtk_cam_dev_request *req, *req_prev;
>         unsigned long flags;
>
>         dev_dbg(cam->dev, "%s\n", __func__);
>
>         spin_lock_irqsave(&cam->pending_job_lock, flags);
>         list_for_each_entry_safe(req, req_prev, &cam->pending_job_list, list)
>                 list_del(&req->list);
>         spin_unlock_irqrestore(&cam->pending_job_lock, flags);
>
>         spin_lock_irqsave(&cam->running_job_lock, flags);
>         list_for_each_entry_safe(req, req_prev, &cam->running_job_list, list)
>                 list_del(&req->list);
>         spin_unlock_irqrestore(&cam->running_job_lock, flags);
> }
>
> static void mtk_cam_vb2_stop_streaming(struct vb2_queue *vq)
> {
>         struct mtk_cam_dev *cam = vb2_get_drv_priv(vq);
>         struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vq);
>         struct device *dev = cam->dev;
>
>         dev_dbg(dev, "%s node:%d count info:%d", __func__,
>                 node->id, atomic_read(&cam->stream_count));
>
>         mutex_lock(&cam->op_lock);
>         if (atomic_read(&cam->stream_count) == cam->enabled_count)
>                 if (v4l2_subdev_call(&cam->subdev, video, s_stream, 0))
>                         dev_err(dev, "failed to stop streaming\n");
>
>         mtk_cam_vb2_return_all_buffers(cam, node, VB2_BUF_STATE_ERROR);
>
>         /* Check the first node to stream-off */
>         if (!atomic_dec_and_test(&cam->stream_count)) {
>                 mutex_unlock(&cam->op_lock);
>                 return;
>         }
>         mutex_unlock(&cam->op_lock);
>
>         mtk_cam_dev_req_cleanup(cam);
>         media_pipeline_stop(&node->vdev.entity);
> }

[keeping the context for Hans]

Best regards,
Tomasz

^ permalink raw reply

* [RFC PATCH 17/17] ARM: dts: aspeed-g5: Sort LPC child nodes by unit address
From: Andrew Jeffery @ 2019-07-26  5:39 UTC (permalink / raw)
  To: linux-aspeed
  Cc: Andrew Jeffery, robh+dt, mark.rutland, joel, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20190726053959.2003-1-andrew@aj.id.au>

Lets try to maintain some sort of sanity.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 arch/arm/boot/dts/aspeed-g5.dtsi | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 50ba58dc5093..99d2995a43db 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -423,17 +423,18 @@
 						status = "disabled";
 					};
 
-					lhc: lhc@20 {
-						compatible = "aspeed,ast2500-lhc";
-						reg = <0x20 0x24 0x48 0x8>;
-					};
-
 					lpc_reset: reset-controller@18 {
 						compatible = "aspeed,ast2500-lpc-reset";
 						reg = <0x18 0x4>;
 						#reset-cells = <1>;
 					};
 
+					lhc: lhc@20 {
+						compatible = "aspeed,ast2500-lhc";
+						reg = <0x20 0x24 0x48 0x8>;
+					};
+
+
 					ibt: ibt@c0 {
 						compatible = "aspeed,ast2500-ibt-bmc";
 						reg = <0xc0 0x18>;
-- 
2.20.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox