* Re: [PATCH v2 0/3] Re-introduce TX FIFO resize for larger EP bursting
From: Wesley Cheng @ 2020-05-21 17:37 UTC (permalink / raw)
To: agross, bjorn.andersson, robh+dt, balbi, gregkh
Cc: linux-arm-msm, devicetree, linux-kernel, linux-usb, jackp
In-Reply-To: <1590050169-30747-1-git-send-email-wcheng@codeaurora.org>
On 5/21/2020 1:36 AM, Wesley Cheng wrote:
> Changes in V2:
> - Modified TXFIFO resizing logic to ensure that each EP is reserved a
> FIFO.
> - Removed dev_dbg() prints and fixed typos from patches
> - Added some more description on the dt-bindings commit message
>
> Reviewed-by: Felipe Balbi <balbi@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
Sorry, please disregard the Reviewed-by tags in the patches. I added
those mistakenly.
> Currently, there is no functionality to allow for resizing the TXFIFOs, and
> relying on the HW default setting for the TXFIFO depth. In most cases, the
> HW default is probably sufficient, but for USB compositions that contain
> multiple functions that require EP bursting, the default settings
> might not be enough. Also to note, the current SW will assign an EP to a
> function driver w/o checking to see if the TXFIFO size for that particular
> EP is large enough. (this is a problem if there are multiple HW defined
> values for the TXFIFO size)
>
> It is mentioned in the SNPS databook that a minimum of TX FIFO depth = 3
> is required for an EP that supports bursting. Otherwise, there may be
> frequent occurences of bursts ending. For high bandwidth functions,
> such as data tethering (protocols that support data aggregation), mass
> storage, and media transfer protocol (over FFS), the bMaxBurst value can be
> large, and a bigger TXFIFO depth may prove to be beneficial in terms of USB
> throughput. (which can be associated to system access latency, etc...) It
> allows for a more consistent burst of traffic, w/o any interruptions, as
> data is readily available in the FIFO.
>
> With testing done using the mass storage function driver, the results show
> that with a larger TXFIFO depth, the bandwidth increased significantly.
>
> Test Parameters:
> - Platform: Qualcomm SM8150
> - bMaxBurst = 6
> - USB req size = 256kB
> - Num of USB reqs = 16
> - USB Speed = Super-Speed
> - Function Driver: Mass Storage (w/ ramdisk)
> - Test Application: CrystalDiskMark
>
> Results:
>
> TXFIFO Depth = 3 max packets
>
> Test Case | Data Size | AVG tput (in MB/s)
> -------------------------------------------
> Sequential|1 GB x |
> Read |9 loops | 193.60
> | | 195.86
> | | 184.77
> | | 193.60
> -------------------------------------------
>
> TXFIFO Depth = 6 max packets
>
> Test Case | Data Size | AVG tput (in MB/s)
> -------------------------------------------
> Sequential|1 GB x |
> Read |9 loops | 287.35
> | | 304.94
> | | 289.64
> | | 293.61
> -------------------------------------------
>
> Wesley Cheng (3):
> usb: dwc3: Resize TX FIFOs to meet EP bursting requirements
> arm64: boot: dts: qcom: sm8150: Enable dynamic TX FIFO resize logic
> dt-bindings: usb: dwc3: Add entry for tx-fifo-resize
>
> Documentation/devicetree/bindings/usb/dwc3.txt | 2 +-
> arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 +
> drivers/usb/dwc3/core.c | 2 +
> drivers/usb/dwc3/core.h | 8 ++
> drivers/usb/dwc3/ep0.c | 37 ++++++++-
> drivers/usb/dwc3/gadget.c | 111 +++++++++++++++++++++++++
> 6 files changed, 159 insertions(+), 2 deletions(-)
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Serge Semin @ 2020-05-21 17:29 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Andy Shevchenko, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521163904.GF4770@sirena.org.uk>
On Thu, May 21, 2020 at 05:39:04PM +0100, Mark Brown wrote:
> On Thu, May 21, 2020 at 07:02:32PM +0300, Andy Shevchenko wrote:
> > On Thu, May 21, 2020 at 6:58 PM Serge Semin
>
> > > Let's leave the patch as is.
>
> > Mark, should I send a partial revert afterwards in this case?
> > I'm not fully satisfied with it.
>
> That might be a suitable way to keep the peace here. You are clearly
> both much more passionate about this choice than I am.
Fine with me.)
-Sergey
^ permalink raw reply
* Re: [PATCH 1/2] spi: dw: add reset control
From: Serge Semin @ 2020-05-21 17:28 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Dinh Nguyen, linux-kernel, devicetree, robh+dt,
linux-spi, Liang Jin J, Andy Shevchenko, Lars Povlsen
In-Reply-To: <20200521171203.GH4770@sirena.org.uk>
On Thu, May 21, 2020 at 06:12:03PM +0100, Mark Brown wrote:
> On Thu, May 21, 2020 at 12:03:58PM -0500, Dinh Nguyen wrote:
> > Add mechanism to get the reset control and deassert it in order to bring
> > the IP out of reset.
> >
> > Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
> > Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>
> Is the From: correct here?
>
> Also adding everyone else who's got patches in flight for this driver
> right now.
>
> > ---
> > drivers/spi/spi-dw-mmio.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > index 384a3ab6dc2d..5c813e15ed89 100644
> > --- a/drivers/spi/spi-dw-mmio.c
> > +++ b/drivers/spi/spi-dw-mmio.c
> > @@ -20,6 +20,7 @@
> > #include <linux/acpi.h>
> > #include <linux/property.h>
> > #include <linux/regmap.h>
> > +#include <linux/reset.h>
> >
> > #include "spi-dw.h"
> >
> > @@ -30,6 +31,7 @@ struct dw_spi_mmio {
> > struct clk *clk;
> > struct clk *pclk;
> > void *priv;
> > + struct reset_control *rstc;
> > };
> >
> > #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL 0x24
> > @@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
> > if (!dwsmmio)
> > return -ENOMEM;
> >
> > + dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");
It would be better to use the optional-version of the reset getter:
devm_reset_control_get_optional_exclusive(), since for instance our DW APB SSI
doesn't have a dedicated reset line.
Note also that you don't need to check the rstc for being NULL. It's null if the
reset line is optional and isn't provided.
If we get to have this patch applied, then DT binding shall be also updated...
Though we haven't got it reviewed yet.(
-Sergey
> > + if (!IS_ERR(dwsmmio->rstc))
> > + reset_control_deassert(dwsmmio->rstc);
> > +
> > dws = &dwsmmio->dws;
> >
> > /* Get basic io resource and map it */
>
> Should we also undo the reset in error paths from probe()? It's not the
> end of the world if we don't but...
>
> > @@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
> > clk_disable_unprepare(dwsmmio->pclk);
> > clk_disable_unprepare(dwsmmio->clk);
> >
> > + if (dwsmmio->rstc)
> > + reset_control_assert(dwsmmio->rstc);
> > +
> > return 0;
> > }
> >
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH v2] arm: dts: am33xx-bone-common: add gpio-line-names
From: Grygorii Strashko @ 2020-05-21 17:26 UTC (permalink / raw)
To: Robert Nelson
Cc: Drew Fustini, Linus Walleij, Benoît Cousson, Tony Lindgren,
Rob Herring, Linux-OMAP, devicetree, linux kernel, Jason Kridner
In-Reply-To: <CAOCHtYiw2jJuzbnW02FUmPy-xmmtErMmow46QQJUMs0VtX=cKg@mail.gmail.com>
On 21/05/2020 17:41, Robert Nelson wrote:
>> Not sure if it should be in am335x-bone-common.dtsi.
>>
>> For example:
>> am335x-boneblack.dts
>> #include "am335x-bone-common.dtsi"
>> #include "am335x-boneblack-common.dtsi" <-- hdmi defined only here
>
> Ah crap, yeah that's a good point.. So if we stick it in...
> am335x-boneblack-common.dtsi
>
> Then the Black-Wireless now has Ethernet...
>
> am335x-boneblack-wireless.dts
> #include "am335x-bone-common.dtsi"
> #include "am335x-boneblack-common.dtsi"
>
> It's going to be ugly, copy and paste mess, but i guess we might as
> well stick it in the device " am335x-boneblack.dts"?
Seems like.
Unfortunately, there is no way to partially overwrite "gpio-line-names"
property.
--
Best regards,
grygorii
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: snps,dw-apb-ssi: add optional reset property
From: Mark Brown @ 2020-05-21 17:13 UTC (permalink / raw)
To: Dinh Nguyen; +Cc: linux-kernel, devicetree, robh+dt, linux-spi
In-Reply-To: <20200521170359.20430-2-dinguyen@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 837 bytes --]
On Thu, May 21, 2020 at 12:03:59PM -0500, Dinh Nguyen wrote:
> Add optional reset property.
>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> index 3ed08ee9feba..e01c557a9876 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> @@ -37,5 +37,7 @@ Example:
> num-cs = <2>;
> cs-gpios = <&gpio0 13 0>,
> <&gpio0 14 0>;
> + resets = <&rst SPIM0_RST>;
> + reset-names = "spi";
This only adds the example, it doesn't add anything to the actual
binding documentation.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] spi: dw: add reset control
From: Mark Brown @ 2020-05-21 17:12 UTC (permalink / raw)
To: Dinh Nguyen
Cc: linux-kernel, devicetree, robh+dt, linux-spi, Liang Jin J,
Serge Semin, fancer.lancer, Andy Shevchenko, Lars Povlsen
In-Reply-To: <20200521170359.20430-1-dinguyen@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]
On Thu, May 21, 2020 at 12:03:58PM -0500, Dinh Nguyen wrote:
> Add mechanism to get the reset control and deassert it in order to bring
> the IP out of reset.
>
> Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
Is the From: correct here?
Also adding everyone else who's got patches in flight for this driver
right now.
> ---
> drivers/spi/spi-dw-mmio.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 384a3ab6dc2d..5c813e15ed89 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -20,6 +20,7 @@
> #include <linux/acpi.h>
> #include <linux/property.h>
> #include <linux/regmap.h>
> +#include <linux/reset.h>
>
> #include "spi-dw.h"
>
> @@ -30,6 +31,7 @@ struct dw_spi_mmio {
> struct clk *clk;
> struct clk *pclk;
> void *priv;
> + struct reset_control *rstc;
> };
>
> #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL 0x24
> @@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
> if (!dwsmmio)
> return -ENOMEM;
>
> + dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");
> + if (!IS_ERR(dwsmmio->rstc))
> + reset_control_deassert(dwsmmio->rstc);
> +
> dws = &dwsmmio->dws;
>
> /* Get basic io resource and map it */
Should we also undo the reset in error paths from probe()? It's not the
end of the world if we don't but...
> @@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
> clk_disable_unprepare(dwsmmio->pclk);
> clk_disable_unprepare(dwsmmio->clk);
>
> + if (dwsmmio->rstc)
> + reset_control_assert(dwsmmio->rstc);
> +
> return 0;
> }
>
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH V4 1/4] media: v4l2-mem2mem: add v4l2_m2m_suspend, v4l2_m2m_resume
From: Tomasz Figa @ 2020-05-21 17:11 UTC (permalink / raw)
To: Jerry-ch Chen, Hans Verkuil
Cc: Hans Verkuil, laurent.pinchart+renesas, matthias.bgg, mchehab,
pihsun, yuzhao, zwisler, linux-mediatek, linux-arm-kernel,
Sean.Cheng, sj.huang, christie.yu, frederic.chen, jungo.lin,
Rynn.Wu, linux-media, srv_heupstream, devicetree, Jerry-ch Chen
In-Reply-To: <20191204124732.10932-2-Jerry-Ch.chen@mediatek.com>
Hi Jerry,
On Wed, Dec 04, 2019 at 08:47:29PM +0800, Jerry-ch Chen wrote:
> From: Pi-Hsun Shih <pihsun@chromium.org>
>
> Add two functions that can be used to stop new jobs from being queued /
> continue running queued job. This can be used while a driver using m2m
> helper is going to suspend / wake up from resume, and can ensure that
> there's no job running in suspend process.
>
> BUG=b:143046833
> TEST=build
>
> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.corp-partner.google.com>
> ---
> drivers/media/v4l2-core/v4l2-mem2mem.c | 40 ++++++++++++++++++++++++++
> include/media/v4l2-mem2mem.h | 22 ++++++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 5bbdec55b7d7..76ba203e0035 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -47,6 +47,10 @@ module_param(debug, bool, 0644);
> #define TRANS_ABORT (1 << 2)
>
>
> +/* The job queue is not running new jobs */
> +#define QUEUE_PAUSED (1 << 0)
> +
> +
> /* Offset base for buffers on the destination queue - used to distinguish
> * between source and destination buffers when mmapping - they receive the same
> * offsets but for different queues */
> @@ -88,6 +92,7 @@ static const char * const m2m_entity_name[] = {
> * @job_queue: instances queued to run
> * @job_spinlock: protects job_queue
> * @job_work: worker to run queued jobs.
> + * @job_queue_flags: flags of the queue status, %QUEUE_PAUSED.
> * @m2m_ops: driver callbacks
> */
> struct v4l2_m2m_dev {
> @@ -105,6 +110,7 @@ struct v4l2_m2m_dev {
> struct list_head job_queue;
> spinlock_t job_spinlock;
> struct work_struct job_work;
> + unsigned long job_queue_flags;
>
> const struct v4l2_m2m_ops *m2m_ops;
> };
> @@ -267,6 +273,12 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
> return;
> }
>
> + if (m2m_dev->job_queue_flags & QUEUE_PAUSED) {
> + spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
> + dprintk("Running new jobs is paused\n");
> + return;
> + }
> +
> m2m_dev->curr_ctx = list_first_entry(&m2m_dev->job_queue,
> struct v4l2_m2m_ctx, queue);
> m2m_dev->curr_ctx->job_flags |= TRANS_RUNNING;
> @@ -447,6 +459,34 @@ void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
> }
> EXPORT_SYMBOL(v4l2_m2m_job_finish);
>
> +void v4l2_m2m_suspend(struct v4l2_m2m_dev *m2m_dev)
> +{
> + unsigned long flags;
> + struct v4l2_m2m_ctx *curr_ctx;
> +
> + spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
> + m2m_dev->job_queue_flags |= QUEUE_PAUSED;
> + curr_ctx = m2m_dev->curr_ctx;
> + spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
> +
> + if (curr_ctx)
> + wait_event(curr_ctx->finished,
> + !(curr_ctx->job_flags & TRANS_RUNNING));
> +}
> +EXPORT_SYMBOL(v4l2_m2m_suspend);
> +
> +void v4l2_m2m_resume(struct v4l2_m2m_dev *m2m_dev)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
> + m2m_dev->job_queue_flags &= ~QUEUE_PAUSED;
> + spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
> +
> + v4l2_m2m_try_run(m2m_dev);
> +}
> +EXPORT_SYMBOL(v4l2_m2m_resume);
> +
> int v4l2_m2m_reqbufs(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
> struct v4l2_requestbuffers *reqbufs)
> {
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 5467264771ec..119a195da390 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -183,6 +183,28 @@ v4l2_m2m_buf_done(struct vb2_v4l2_buffer *buf, enum vb2_buffer_state state)
> vb2_buffer_done(&buf->vb2_buf, state);
> }
>
> +/**
> + * v4l2_m2m_suspend() - stop new jobs from being run and wait for current job
> + * to finish
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * Called by a driver in the suspend hook. Stop new jobs from being run, and
> + * wait for current running job to finish.
> + */
> +void v4l2_m2m_suspend(struct v4l2_m2m_dev *m2m_dev);
> +
> +/**
> + * v4l2_m2m_resume() - resume job running and try to run a queued job
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * Called by a driver in the resume hook. This reverts the operation of
> + * v4l2_m2m_suspend() and allows job to be run. Also try to run a queued job if
> + * there is any.
> + */
> +void v4l2_m2m_resume(struct v4l2_m2m_dev *m2m_dev);
> +
> /**
> * v4l2_m2m_reqbufs() - multi-queue-aware REQBUFS multiplexer
> *
> --
> 2.18.0
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
[Corrected Hans's email address.]
Hans, does this look good to you?
Best regards,
Tomasz
^ permalink raw reply
* [PATCH 2/2] dt-bindings: snps,dw-apb-ssi: add optional reset property
From: Dinh Nguyen @ 2020-05-21 17:03 UTC (permalink / raw)
To: linux-kernel; +Cc: dinguyen, devicetree, broonie, robh+dt, linux-spi
In-Reply-To: <20200521170359.20430-1-dinguyen@kernel.org>
Add optional reset property.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index 3ed08ee9feba..e01c557a9876 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -37,5 +37,7 @@ Example:
num-cs = <2>;
cs-gpios = <&gpio0 13 0>,
<&gpio0 14 0>;
+ resets = <&rst SPIM0_RST>;
+ reset-names = "spi";
};
--
2.17.1
^ permalink raw reply related
* [PATCH 1/2] spi: dw: add reset control
From: Dinh Nguyen @ 2020-05-21 17:03 UTC (permalink / raw)
To: linux-kernel
Cc: dinguyen, devicetree, broonie, robh+dt, linux-spi, Liang Jin J
Add mechanism to get the reset control and deassert it in order to bring
the IP out of reset.
Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
drivers/spi/spi-dw-mmio.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 384a3ab6dc2d..5c813e15ed89 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -20,6 +20,7 @@
#include <linux/acpi.h>
#include <linux/property.h>
#include <linux/regmap.h>
+#include <linux/reset.h>
#include "spi-dw.h"
@@ -30,6 +31,7 @@ struct dw_spi_mmio {
struct clk *clk;
struct clk *pclk;
void *priv;
+ struct reset_control *rstc;
};
#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL 0x24
@@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
if (!dwsmmio)
return -ENOMEM;
+ dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");
+ if (!IS_ERR(dwsmmio->rstc))
+ reset_control_deassert(dwsmmio->rstc);
+
dws = &dwsmmio->dws;
/* Get basic io resource and map it */
@@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
clk_disable_unprepare(dwsmmio->pclk);
clk_disable_unprepare(dwsmmio->clk);
+ if (dwsmmio->rstc)
+ reset_control_assert(dwsmmio->rstc);
+
return 0;
}
--
2.17.1
^ permalink raw reply related
* RE: [PATCH v9 10/10] arm64: dts: Add node for ufs exynos7
From: Alim Akhtar @ 2020-05-21 16:49 UTC (permalink / raw)
To: 'Krzysztof Kozlowski'
Cc: robh, devicetree, linux-scsi, avri.altman, martin.petersen,
kwmad.kim, stanley.chu, cang, linux-samsung-soc, linux-arm-kernel,
linux-kernel
In-Reply-To: <20200519071636.GA6971@kozik-lap>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 19 May 2020 12:47
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: robh@kernel.org; devicetree@vger.kernel.org; linux-scsi@vger.kernel.org;
> avri.altman@wdc.com; martin.petersen@oracle.com;
> kwmad.kim@samsung.com; stanley.chu@mediatek.com;
> cang@codeaurora.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 10/10] arm64: dts: Add node for ufs exynos7
>
> On Thu, May 14, 2020 at 06:09:14AM +0530, Alim Akhtar wrote:
> > Adding dt node foe UFS and UFS-PHY for exynos7 SoC.
> >
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Tested-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > ---
> > .../boot/dts/exynos/exynos7-espresso.dts | 4 ++
> > arch/arm64/boot/dts/exynos/exynos7.dtsi | 43 ++++++++++++++++++-
> > 2 files changed, 45 insertions(+), 2 deletions(-)
>
> I will pick it up after all bindings get Rob's ack (or are picked up as well). The
> second bindings patch are still pending on that.
>
Thank Krzysztof,
Yes, one binding still awaiting Rob's ack, I have addressed his comment in this v9 series.
Hoping he will find some time to review the same.
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Andy Shevchenko @ 2020-05-21 16:44 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521163904.GF4770@sirena.org.uk>
On Thu, May 21, 2020 at 7:39 PM Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 21, 2020 at 07:02:32PM +0300, Andy Shevchenko wrote:
> > On Thu, May 21, 2020 at 6:58 PM Serge Semin
>
> > > Let's leave the patch as is.
>
> > Mark, should I send a partial revert afterwards in this case?
> > I'm not fully satisfied with it.
>
> That might be a suitable way to keep the peace here. You are clearly
> both much more passionate about this choice than I am.
Okay, will work for me, thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Mark Brown @ 2020-05-21 16:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <CAHp75VcY9HG3=wft9VFWKJD9PG6TiSbmOyjgYZyue+z3PW_eCQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 371 bytes --]
On Thu, May 21, 2020 at 07:02:32PM +0300, Andy Shevchenko wrote:
> On Thu, May 21, 2020 at 6:58 PM Serge Semin
> > Let's leave the patch as is.
> Mark, should I send a partial revert afterwards in this case?
> I'm not fully satisfied with it.
That might be a suitable way to keep the peace here. You are clearly
both much more passionate about this choice than I am.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
From: Evan Green @ 2020-05-21 16:19 UTC (permalink / raw)
To: Rakesh Pillai
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-arm Mailing List, LKML, linux-arm-msm
In-Reply-To: <1589946996-31264-1-git-send-email-pillair@codeaurora.org>
On Tue, May 19, 2020 at 8:57 PM Rakesh Pillai <pillair@codeaurora.org> wrote:
>
> Add device node for the ath10k SNOC platform driver probe
> and add resources required for WCN3990 on sc7180 soc.
>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
^ permalink raw reply
* Re: [PATCH v8 14/14] media: platform: Add jpeg dec/enc feature
From: Tomasz Figa @ 2020-05-21 16:08 UTC (permalink / raw)
To: Hans Verkuil, Xia Jiang
Cc: Xia Jiang, Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Rick Chang, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, Marek Szyprowski,
srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
sj.huang
In-Reply-To: <b62b303c-10cd-fdf6-52fa-90d63124487a@xs4all.nl>
On Mon, May 11, 2020 at 11:04:19AM +0200, Hans Verkuil wrote:
> On 03/04/2020 11:40, Xia Jiang wrote:
> > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg
> > decode and encode have great similarities with function operation.
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8:jpeg encoder and decoder use separate callbacks instead of repeating
> > the if/else in every callback.
> > improve vidioc_try_fmt() implementation that can be shared by jpeg
> > encoder and decoder.
> > fix the bug of jpeg encoder s_selection implementation.
> > cancel the state of the jpeg encoder.
> > improve jpeg encoder and decoder set default params flow.
> > put the clock names and other datas in a match_data struct.
> > fix the bug of geting correctly quality value.
> > do the all the bits' settings of one register in one function.
> > move the code of mtk_jpeg_enc_reg.h to mtk_jpeg_enc_hw.h and delete
> > mtk_jpeg_enc_reg.h.
> >
> > v7: reverse spin lock and unlock operation in device run function for
> > multi-instance.
> >
> > v6: add space to arounding '+'.
> > alignment 'struct mtk_jpeg_fmt *fmt' match open parenthesis.
> > change 'mtk_jpeg_enc_set_encFormat' to 'mtk_jpeg_enc_set_enc_format'.
> > make 'mtk_jpeg_ctrls_setup' to static prototype.
> > delete unused variables 'jpeg'/'align_h'/'align_w'/'flags'.
> > initialize 'yuv_format'/'enc_quality' variables.
> >
> > v5: support crop for encoder and compose for decoder in s_selection and
> > g_selection function.
> > use clamp() to replace mtk_jpeg_bound_align_image() and round_up()
> > to replace mtk_jpeg_align().
> > delete jpeg_enc_param/mtk_jpeg_enc_param structure and
> > mtk_jpeg_set_param(), program the registers directly based on
> > the original V4L2 values.
> > move macro definition about hw to mtk_jpeg_enc_reg.h.
> > delete unnecessary V4L2 logs in driver.
> > cancel spin lock and unlock operation in deviec run function.
> > change jpeg enc register offset hex numberals from upercase to lowercase.
> >
> > v4: split mtk_jpeg_try_fmt_mplane() to two functions, one for encoder,
> > one for decoder.
> > split mtk_jpeg_set_default_params() to two functions, one for
> > encoder, one for decoder.
> > add cropping support for encoder in g/s_selection ioctls.
> > change exif mode support by using V4L2_JPEG_ACTIVE_MARKER_APP1.
> > change MTK_JPEG_MAX_WIDTH/MTK_JPEG_MAX_HEIGH from 8192 to 65535 by
> > specification.
> > move width shifting operation behind aligning operation in
> > mtk_jpeg_try_enc_fmt_mplane() for bug fix.
> > fix user abuseing data_offset issue for DMABUF in
> > mtk_jpeg_set_enc_src().
> > fix kbuild warings: change MTK_JPEG_MIN_HEIGHT/MTK_JPEG_MAX_HEIGHT
> > and MTK_JPEG_MIN_WIDTH/MTK_JPEG_MAX_WIDTH from
> > 'int' type to 'unsigned int' type.
> > fix msleadingly indented of 'else'.
> > v3: delete Change-Id.
> > only test once handler->error after the last v4l2_ctrl_new_std().
> > seperate changes of v4l2-ctrls.c and v4l2-controls.h to new patch.
> > v2: fix compliance test fail, check created buffer size in driver.
> > ---
> > drivers/media/platform/mtk-jpeg/Makefile | 5 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 1038 +++++++++++++----
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 51 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h | 7 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 193 +++
> > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h | 123 ++
> > 6 files changed, 1188 insertions(+), 229 deletions(-)
> > create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile
> > index 48516dcf96e6..76c33aad0f3f 100644
> > --- a/drivers/media/platform/mtk-jpeg/Makefile
> > +++ b/drivers/media/platform/mtk-jpeg/Makefile
> > @@ -1,3 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_dec_hw.o mtk_jpeg_dec_parse.o
> > +mtk_jpeg-objs := mtk_jpeg_core.o \
> > + mtk_jpeg_dec_hw.o \
> > + mtk_jpeg_dec_parse.o \
> > + mtk_jpeg_enc_hw.o
> > obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 77a95185584c..18a759ce2c46 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -3,6 +3,7 @@
> > * Copyright (c) 2016 MediaTek Inc.
> > * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > * Rick Chang <rick.chang@mediatek.com>
> > + * Xia Jiang <xia.jiang@mediatek.com>
> > */
> >
> > #include <linux/clk.h>
> > @@ -23,11 +24,60 @@
> > #include <media/videobuf2-dma-contig.h>
> > #include <soc/mediatek/smi.h>
> >
> > +#include "mtk_jpeg_enc_hw.h"
> > #include "mtk_jpeg_dec_hw.h"
> > #include "mtk_jpeg_core.h"
> > #include "mtk_jpeg_dec_parse.h"
> >
> > -static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > +static struct mtk_jpeg_fmt mtk_jpeg_enc_formats[] = {
> > + {
> > + .fourcc = V4L2_PIX_FMT_JPEG,
> > + .colplanes = 1,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_CAPTURE,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_NV12M,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_NV12,
> > + .h_sample = {4, 4},
> > + .v_sample = {4, 2},
> > + .colplanes = 2,
> > + .h_align = 4,
> > + .v_align = 4,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_NV21M,
> > + .hw_format = JEPG_ENC_YUV_FORMAT_NV21,
> > + .h_sample = {4, 4},
> > + .v_sample = {4, 2},
> > + .colplanes = 2,
> > + .h_align = 4,
> > + .v_align = 4,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_YUYV,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_YUYV,
> > + .h_sample = {8},
> > + .v_sample = {4},
> > + .colplanes = 1,
> > + .h_align = 5,
> > + .v_align = 3,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_YVYU,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_YVYU,
> > + .h_sample = {8},
> > + .v_sample = {4},
> > + .colplanes = 1,
> > + .h_align = 5,
> > + .v_align = 3,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > +};
> > +
> > +static struct mtk_jpeg_fmt mtk_jpeg_dec_formats[] = {
> > {
> > .fourcc = V4L2_PIX_FMT_JPEG,
> > .colplanes = 1,
> > @@ -53,7 +103,8 @@ static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > },
> > };
> >
> > -#define MTK_JPEG_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_formats)
> > +#define MTK_JPEG_ENC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_enc_formats)
> > +#define MTK_JPEG_DEC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_dec_formats)
> >
> > enum {
> > MTK_JPEG_BUF_FLAGS_INIT = 0,
> > @@ -70,6 +121,11 @@ struct mtk_jpeg_src_buf {
> > static int debug;
> > module_param(debug, int, 0644);
> >
> > +static inline struct mtk_jpeg_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> > +{
> > + return container_of(ctrl->handler, struct mtk_jpeg_ctx, ctrl_hdl);
> > +}
> > +
> > static inline struct mtk_jpeg_ctx *mtk_jpeg_fh_to_ctx(struct v4l2_fh *fh)
> > {
> > return container_of(fh, struct mtk_jpeg_ctx, fh);
> > @@ -81,12 +137,25 @@ static inline struct mtk_jpeg_src_buf *mtk_jpeg_vb2_to_srcbuf(
> > return container_of(to_vb2_v4l2_buffer(vb), struct mtk_jpeg_src_buf, b);
> > }
> >
> > -static int mtk_jpeg_querycap(struct file *file, void *priv,
> > - struct v4l2_capability *cap)
> > +static int mtk_jpeg_enc_querycap(struct file *file, void *priv,
> > + struct v4l2_capability *cap)
> > +{
> > + struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +
> > + strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > + strscpy(cap->card, MTK_JPEG_NAME " encoder", sizeof(cap->card));
> > + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > + dev_name(jpeg->dev));
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_querycap(struct file *file, void *priv,
> > + struct v4l2_capability *cap)
> > {
> > struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> >
> > - strscpy(cap->driver, MTK_JPEG_NAME " decoder", sizeof(cap->driver));
> > + strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > strscpy(cap->card, MTK_JPEG_NAME " decoder", sizeof(cap->card));
> > snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > dev_name(jpeg->dev));
> > @@ -94,6 +163,54 @@ static int mtk_jpeg_querycap(struct file *file, void *priv,
> > return 0;
> > }
> >
> > +static int vidioc_jpeg_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct mtk_jpeg_ctx *ctx = ctrl_to_ctx(ctrl);
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_JPEG_RESTART_INTERVAL:
> > + ctx->restart_interval = ctrl->val;
> > + break;
> > + case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> > + ctx->enc_quality = ctrl->val;
> > + break;
> > + case V4L2_CID_JPEG_ACTIVE_MARKER:
> > + ctx->enable_exif = ctrl->val & V4L2_JPEG_ACTIVE_MARKER_APP1 ?
> > + true : false;
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_jpeg_enc_ctrl_ops = {
> > + .s_ctrl = vidioc_jpeg_enc_s_ctrl,
> > +};
> > +
> > +static int mtk_jpeg_enc_ctrls_setup(struct mtk_jpeg_ctx *ctx)
> > +{
> > + const struct v4l2_ctrl_ops *ops = &mtk_jpeg_enc_ctrl_ops;
> > + struct v4l2_ctrl_handler *handler = &ctx->ctrl_hdl;
> > +
> > + v4l2_ctrl_handler_init(handler, 3);
> > +
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_RESTART_INTERVAL, 0, 100,
> > + 1, 0);
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_COMPRESSION_QUALITY, 48,
> > + 100, 1, 90);
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_ACTIVE_MARKER, 0,
> > + V4L2_JPEG_ACTIVE_MARKER_APP1, 0, 0);
> > +
> > + if (handler->error) {
> > + v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> > + return handler->error;
> > + }
> > +
> > + v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
> > +
> > + return 0;
> > +}
> > +
> > static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> > struct v4l2_fmtdesc *f, u32 type)
> > {
> > @@ -115,117 +232,105 @@ static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> > return 0;
> > }
> >
> > -static int mtk_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
> > - struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_cap(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > {
> > - return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > + MTK_JPEG_ENC_NUM_FORMATS, f,
> > + MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_cap(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats,
> > + MTK_JPEG_DEC_NUM_FORMATS, f,
> > MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > }
> >
> > -static int mtk_jpeg_enum_fmt_vid_out(struct file *file, void *priv,
> > - struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_out(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > + MTK_JPEG_ENC_NUM_FORMATS, f,
> > + MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_out(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > {
> > - return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > - MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats, MTK_JPEG_DEC_NUM_FORMATS,
> > + f, MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > }
>
> OK, so this patch is very hard to read because there are two independent changes
> taking place:
>
> 1) rename existing functions/defines/variables with a _dec prefix to prepare
> for the addition of the encoder feature.
>
> 2) add the encoder feature.
>
> Please split up this patch into two parts: one that does the rename and as much of
> the preparation to support both decoder and encoder without changing the
> functionality, and a second one that actually adds the new encoder feature.
>
> In fact, once that's done it is likely that most of this patch series can be
> merged, even if there are still things that need to be changed for the last
> patch adding the encoder support. I see nothing objectionable in patches 1-10
> and 13. So merging those together with a new rename patch wouldn't be an issue,
> I think.
>
> In any case, the diffs should be a lot cleaner and easier to review by splitting
> it up like that.
Agreed with Hans. The split will make it much easier to review this one
for me, so I'll wait for it. Thanks in advance.
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Andy Shevchenko @ 2020-05-21 16:02 UTC (permalink / raw)
To: Serge Semin
Cc: Mark Brown, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521155804.qgaaqc23h2nll7r3@mobilestation>
On Thu, May 21, 2020 at 6:58 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
> On Thu, May 21, 2020 at 04:51:43PM +0100, Mark Brown wrote:
> > On Thu, May 21, 2020 at 03:12:28PM +0300, Serge Semin wrote:
> >
> > > Well, for me both solutions are equal except mine consumes less stack memory.
> > > The only reason why your solution might be better is that if DW DMA driver or
> > > the DMA engine subsystem changed the dw_dma_slave structure instance passed to
> > > the dma_request_channel() method, which non of them do. So I'll leave this for
> > > Mark to decide. Mark, could you give us your final word about this?
> >
> > Honestly I'm struggling to care either way. I guess saving a bit of
> > stack is potentially useful.
>
> Settled then.
With whom?
> Let's leave the patch as is.
Mark, should I send a partial revert afterwards in this case?
I'm not fully satisfied with it.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v8 13/14] media: platform: Rename jpeg dec file name
From: Tomasz Figa @ 2020-05-21 16:02 UTC (permalink / raw)
To: Xia Jiang
Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
Matthias Brugger, Rick Chang, linux-media, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
sj.huang
In-Reply-To: <20200403094033.8288-14-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:32PM +0800, Xia Jiang wrote:
> Rename the files which are for decode feature. This is preparing
> path since the jpeg enc patch will be added later.
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
>
> v7: no changes
>
> v6: no changes
>
> v5: no changes
>
> v4: no changes
>
> v3: no changes
>
> v2: no changes
> ---
> drivers/media/platform/mtk-jpeg/Makefile | 2 +-
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++--
> .../platform/mtk-jpeg/{mtk_jpeg_hw.c => mtk_jpeg_dec_hw.c} | 2 +-
> .../platform/mtk-jpeg/{mtk_jpeg_hw.h => mtk_jpeg_dec_hw.h} | 2 +-
> .../mtk-jpeg/{mtk_jpeg_parse.c => mtk_jpeg_dec_parse.c} | 2 +-
> .../mtk-jpeg/{mtk_jpeg_parse.h => mtk_jpeg_dec_parse.h} | 2 +-
> .../platform/mtk-jpeg/{mtk_jpeg_reg.h => mtk_jpeg_dec_reg.h} | 0
> 7 files changed, 7 insertions(+), 7 deletions(-)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_hw.c => mtk_jpeg_dec_hw.c} (99%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_hw.h => mtk_jpeg_dec_hw.h} (98%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_parse.c => mtk_jpeg_dec_parse.c} (98%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_parse.h => mtk_jpeg_dec_parse.h} (92%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_reg.h => mtk_jpeg_dec_reg.h} (100%)
>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH v8 11/14] media: dt-bindings: Add jpeg enc device tree node document
From: Tomasz Figa @ 2020-05-21 16:00 UTC (permalink / raw)
To: Xia Jiang
Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
Matthias Brugger, Rick Chang, linux-media, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
sj.huang, yong.wu
In-Reply-To: <20200403094033.8288-12-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:30PM +0800, Xia Jiang wrote:
> Add jpeg enc device tree node document
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
>
> v7: no changes
>
> v6: no changes
>
> v5: no changes
>
> v4: no changes
>
> v3: change compatible to SoC specific compatible
>
> v2: no changes
> ---
> .../bindings/media/mediatek-jpeg-encoder.txt | 37 +++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
>
Thank you for the patch. Please see my comments inline.
> diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
> new file mode 100644
> index 000000000000..fa8da699493b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
> @@ -0,0 +1,37 @@
> +* MediaTek JPEG Encoder
> +
> +MediaTek JPEG Encoder is the JPEG encode hardware present in MediaTek SoCs
> +
> +Required properties:
> +- compatible : should be one of:
> + "mediatek,mt2701-jpgenc"
> + ...
What does this "..." mean?
> + followed by "mediatek,mtk-jpgenc"
> +- reg : physical base address of the JPEG encoder registers and length of
> + memory mapped region.
> +- interrupts : interrupt number to the interrupt controller.
> +- clocks: device clocks, see
> + Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> +- clock-names: must contain "jpgenc". It is the clock of JPEG encoder.
nit: In principle the clocks should be named after the function the clock
performs on the consumer side, i.e. the JPEG block in this case, I guess
here it's just a generic clock that does everything, but I guess it comes
from somewhere. Is it the AHB clock or something? In that case it would
normally be called "ahb".
> +- power-domains: a phandle to the power domain, see
> + Documentation/devicetree/bindings/power/power_domain.txt for details.
> +- mediatek,larb: must contain the local arbiters in the current SoCs, see
> + Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> + for details.
I believe this isn't necessary anymore, because larbs are added
automatically by the MTK IOMMU driver using device links. +CC Yong who
worked on that.
> +- iommus: should point to the respective IOMMU block with master port as
> + argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> + for details.
> +
> +Example:
> + jpegenc: jpegenc@1500a000 {
> + compatible = "mediatek,mt2701-jpgenc",
> + "mediatek,mtk-jpgenc";
> + reg = <0 0x1500a000 0 0x1000>;
> + interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&imgsys CLK_IMG_VENC>;
> + clock-names = "jpgenc";
> + power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
> + mediatek,larb = <&larb2>;
Ditto.
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH V6 2/7] soc: qcom-geni-se: Add interconnect support to fix earlycon crash
From: Matthias Kaehlcke @ 2020-05-21 16:00 UTC (permalink / raw)
To: Akash Asthana
Cc: gregkh, agross, bjorn.andersson, wsa, broonie, mark.rutland,
robh+dt, linux-i2c, linux-spi, devicetree, swboyd, mgautam,
linux-arm-msm, linux-serial, dianders, msavaliy, evgreen
In-Reply-To: <1590049764-20912-3-git-send-email-akashast@codeaurora.org>
Hi Akash,
On Thu, May 21, 2020 at 01:59:19PM +0530, Akash Asthana wrote:
> QUP core clock is shared among all the SE drivers present on particular
> QUP wrapper, the system will reset(unclocked access) if earlycon used after
> QUP core clock is put to 0 from other SE drivers before real console comes
> up.
>
> As earlycon can't vote for it's QUP core need, to fix this add ICC
> support to common/QUP wrapper driver and put vote for QUP core from
> probe on behalf of earlycon and remove vote during earlycon exit call.
>
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> Reported-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Change in V3:
> - Add geni_remove_earlycon_icc_vote API that will be used by earlycon
> exit function to remove ICC vote for earlyconsole.
> - Remove suspend/resume hook for geni-se driver as we are no longer
> removing earlyconsole ICC vote from system suspend, we are removing
> from earlycon exit.
>
> Change in V4:
> - As per Matthias comment make 'earlycon_wrapper' as static structure.
>
> Changes in V5:
> - Vote for core path only after checking whether "qcom_geni" earlycon is
> actually present or not by traversing over structure "console_drivers".
>
> Changes in V6:
> - As per Matthias's comment removed NULL check for console_drivers global
> struct, added NULL check for earlycon_wrapper in _remove_earlycon_icc_vote
> API
> - Addressed nitpicks from Andy.
>
> drivers/soc/qcom/qcom-geni-se.c | 68 +++++++++++++++++++++++++++++++++++
> drivers/tty/serial/qcom_geni_serial.c | 7 ++++
> include/linux/qcom-geni-se.h | 2 ++
> 3 files changed, 77 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 0b2526d..ac16bb1 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -3,6 +3,7 @@
>
> #include <linux/acpi.h>
> #include <linux/clk.h>
> +#include <linux/console.h>
> #include <linux/slab.h>
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> @@ -90,11 +91,14 @@ struct geni_wrapper {
> struct device *dev;
> void __iomem *base;
> struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> + struct geni_icc_path to_core;
> };
>
> static const char * const icc_path_names[] = {"qup-core", "qup-config",
> "qup-memory"};
>
> +static struct geni_wrapper *earlycon_wrapper;
> +
> #define QUP_HW_VER_REG 0x4
>
> /* Common SE registers */
> @@ -812,11 +816,38 @@ int geni_icc_disable(struct geni_se *se)
> }
> EXPORT_SYMBOL(geni_icc_disable);
>
> +void geni_remove_earlycon_icc_vote(void)
> +{
> + struct geni_wrapper *wrapper;
> + struct device_node *parent;
> + struct device_node *child;
> +
> + if (!earlycon_wrapper)
> + return;
> +
> + wrapper = earlycon_wrapper;
> + parent = of_get_next_parent(wrapper->dev->of_node);
> + for_each_child_of_node(parent, child) {
> + if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
> + continue;
> + wrapper = platform_get_drvdata(of_find_device_by_node(child));
> + icc_put(wrapper->to_core.path);
> + wrapper->to_core.path = NULL;
> +
> + }
> + of_node_put(parent);
> +
> + earlycon_wrapper = NULL;
> +}
> +EXPORT_SYMBOL(geni_remove_earlycon_icc_vote);
> +
> static int geni_se_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct resource *res;
> struct geni_wrapper *wrapper;
> + struct console __maybe_unused *bcon;
> + bool __maybe_unused has_earlycon = false;
> int ret;
>
> wrapper = devm_kzalloc(dev, sizeof(*wrapper), GFP_KERNEL);
> @@ -839,6 +870,43 @@ static int geni_se_probe(struct platform_device *pdev)
> }
> }
>
> +#ifdef CONFIG_SERIAL_EARLYCON
> + for_each_console(bcon) {
> + if (!strcmp(bcon->name, "qcom_geni")) {
> + has_earlycon = true;
> + break;
> + }
> + }
> + if (!has_earlycon)
> + goto exit;
> +
> + wrapper->to_core.path = devm_of_icc_get(dev, "qup-core");
> + if (IS_ERR(wrapper->to_core.path))
> + return PTR_ERR(wrapper->to_core.path);
> + /*
> + * Put minmal BW request on core clocks on behalf of early console.
> + * The vote will be removed earlycon exit function.
> + *
> + * Note: We are putting vote on each QUP wrapper instead only to which
> + * earlycon is connected because QUP core clock of different wrapper
> + * share same voltage domain. If core1 is put to 0, then core2 will
> + * also run at 0, if not voted. Default ICC vote will be removed ASA
> + * we touch any of the core clock.
> + * core1 = core2 = max(core1, core2)
> + */
> + ret = icc_set_bw(wrapper->to_core.path, GENI_DEFAULT_BW,
> + GENI_DEFAULT_BW);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: ICC BW voting failed for core :%d\n",
nit: " ... core: %d\n".
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply
* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Serge Semin @ 2020-05-21 15:58 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Andy Shevchenko, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521155143.GE4770@sirena.org.uk>
On Thu, May 21, 2020 at 04:51:43PM +0100, Mark Brown wrote:
> On Thu, May 21, 2020 at 03:12:28PM +0300, Serge Semin wrote:
>
> > Well, for me both solutions are equal except mine consumes less stack memory.
> > The only reason why your solution might be better is that if DW DMA driver or
> > the DMA engine subsystem changed the dw_dma_slave structure instance passed to
> > the dma_request_channel() method, which non of them do. So I'll leave this for
> > Mark to decide. Mark, could you give us your final word about this?
>
> Honestly I'm struggling to care either way. I guess saving a bit of
> stack is potentially useful.
Settled then. Let's leave the patch as is. I suppose we've finally finished a
review except a question Feng asked to the patch:
[PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA
If you are ok with my responses, then the patchset is ready for you further
actions.
-Sergey
^ permalink raw reply
* Re: [PATCH 0/4] Update K3 DSP remoteproc driver for C71x DSPs
From: Suman Anna @ 2020-05-21 15:57 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Mathieu Poirier
Cc: Clement Leger, Loic Pallardy, Arnaud Pouliquen, Lokesh Vutla,
linux-remoteproc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20200325204701.16862-1-s-anna@ti.com>
On 3/25/20 3:46 PM, Suman Anna wrote:
> Hi All,
>
> This series adds support for a new next generation 64-bit TI DSP based on
> the TMS320C71x CorePac processor subsystem called the C71x. The support is
> enabled through couple of enhancements to the remoteproc core (primarily to
> support a 64-bit trace resource entry), and does depend on the K3 DSP
> remoteproc driver posted earlier today [1].
>
> The loading support leveraged the 64-bit ELF loader support code added by
> Clement and already staged on the rproc-next branch. I am posting this
> series separate from the C66x series because of the new 64-bit resource
> type enhancement needs (patches 2 and 3). I have leveraged the existing
> resource types as is by introducing a new version element, and am open to
> ideas if it is desired to just define it as a separate resource type.
>
> The C71x DSP boots using firmware segments loaded into the DDR with a 2 MB
> aligned address requirement on the boot vectors. There is no support for
> internal memory loading, and all internal memories shall be used as fast
> RAMs/scatchpads by the firmware executing on the DSPs. IPC is through the
> virtio-rpmsg transport. There is no support for Error Recovery, Power
> Management or loading into on-chip SRAMs at present.
>
> Following is the patch summary:
> - Patch 1 updates the K3 DSP bindings for C71x cores
> - Patch 2 introduces a concept of version element into existing resource types
> - Patch 3 adds support for a new 64-bit trace resource entry
> - Patch 4 enhances the K3 DSP remoteproc driver for C71x
I have separated out the C71 platform driver pieces (patches 1 & 4) and
posted a v2 for those.
Appreciate any feedback on the core patches (patches 2 & 3) that add the
minimal 64-bit trace support, as this also sets the direction for
resource extensions. I can post the next version for those based on
feedback.
regards
Suman
>
> regards
> Suman
>
> [1] https://patchwork.kernel.org/cover/11458573/
>
> Suman Anna (4):
> dt-bindings: remoteproc: k3-dsp: Update bindings for C71x DSPs
> remoteproc: introduce version element into resource type field
> remoteproc: add support for a new 64-bit trace version
> remoteproc/k3-dsp: Add support for C71x DSPs
>
> .../bindings/remoteproc/ti,k3-dsp-rproc.yaml | 78 ++++++++++++++++---
> drivers/remoteproc/remoteproc_core.c | 65 +++++++++++-----
> drivers/remoteproc/remoteproc_debugfs.c | 50 ++++++++----
> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 17 ++++
> include/linux/remoteproc.h | 34 +++++++-
> 5 files changed, 203 insertions(+), 41 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Andy Shevchenko @ 2020-05-21 15:56 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521155143.GE4770@sirena.org.uk>
On Thu, May 21, 2020 at 6:51 PM Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 21, 2020 at 03:12:28PM +0300, Serge Semin wrote:
>
> > Well, for me both solutions are equal except mine consumes less stack memory.
> > The only reason why your solution might be better is that if DW DMA driver or
> > the DMA engine subsystem changed the dw_dma_slave structure instance passed to
> > the dma_request_channel() method, which non of them do. So I'll leave this for
> > Mark to decide. Mark, could you give us your final word about this?
>
> Honestly I'm struggling to care either way. I guess saving a bit of
> stack is potentially useful.
Yes, but OTOH dropping maintainability by this is worse in my opinion.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Srinivas Kandagatla @ 2020-05-21 15:55 UTC (permalink / raw)
To: Doug Anderson
Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <CAD=FV=W+UES1f3reMhvPPUho5FbaZXdU-2jkRaPcbBEzDWT+WQ@mail.gmail.com>
On 21/05/2020 16:10, Doug Anderson wrote:
>> On 20/05/2020 23:48, Doug Anderson wrote:
>>>> Is this only applicable for corrected address space?
>>> I guess I was proposing a two dts-node / two drive approach here.
>>>
>>> dts node #1:just covers the memory range for accessing the FEC-corrected data
>>> driver #1: read-only and reads the FEC-corrected data
>>>
>>> dts node #2: covers the memory range that's_not_ the FEC-corrected
>>> memory range.
>>> driver #2: read-write. reading reads uncorrected data
>>>
>>> Does that seem sane?
>> I see your point but it does not make sense to have two node for same thing.
> OK, so that sounds as if we want to go with the proposal where we
> "deprecate the old driver and/or bindings and say that there really
> should just be one node and one driver".
>
> Would this be acceptable to you?
>
> 1. Officially mark the old bindings as deprecated.
Possibly Yes for some reasons below!
>
> 2. Leave the old driver there to support the old deprecated bindings,
> at least until everyone can be transferred over. There seem to be
> quite a few existing users of "qcom,qfprom" and we're supposed to make
> an attempt at keeping the old device trees working, at least for a
> little while. Once everyone is transferred over we could decide to
> delete the old driver.
we could consider "qcom,qfrom" to be only passing corrected address
space. Till we transition users to new bindings!
>
Yes.
> 3. We will have a totally new driver here.
No, we should still be using the same driver. But the exiting driver
seems to incorrect and is need of fixing.
Having a look at the memory map for old SoCs like msm8996 and msm8916
shows that memory map that was passed to qfprom driver is corrected
address space. Writes will not obviously work!
This should also be true with sdm845 or sc7180
That needs to be fixed first!
>
> 4. A given device tree will_not_ be allowed to have both
> "qcom,qfprom" specified and "qcom,SOC-qfprom" specified. ...and by
> "qcom,SOC-qfprom" I mean that SOC should be replaced by the SoC name,
> so "qcom,sc7180-qfprom" or "qcom,sdm845-qfprom". So once you switch
> to the new node it replaces the old node.
Secondly, this IP is clearly an integral part of Secure Control Block,
which clearly has versioning information.
Versioning information should be part of compatible string in msm8996 it
should be "qcom,qfprom-5.1.0"
for msm8916 it should be "qcom,qfprom-4.0.0" this translates to
"qcom,qfprom-<MAJOR-NUMBER>-<MINOR-NUMBER>-<STEP>"
Thirdly we should be able to have common read for all these as they tend
to just read from corrected address space.
Offsets to corrected address space seems to always constant across SoCs too.
platform specific device tree nodes should also be able to specify
"read-only" property to not allow writes on to this raw area.
Does this make sense ?
Thanks,
srini
>
>
>> Isn't the raw address space reads used to for blowing and checking the
>> fuses if they are blown correctly or not and software usage of these
>> fuses should only be done from correct address space?
>>
>> the read interface to user should be always from corrected address space
>> and write interface should be to raw address space.
> Great. That sounds right to me. Presumably the driver could add some
> sort of "debugfs" access to read the raw address space if needed.
^ permalink raw reply
* Re: [PATCHv3 2/2] dt-bindings: arm: coresight: Add support to skip trace unit power up
From: Mathieu Poirier @ 2020-05-21 15:53 UTC (permalink / raw)
To: Sai Prakash Ranjan
Cc: Suzuki K Poulose, Mike Leach, devicetree, Rob Herring,
Stephen Boyd, Leo Yan, linux-arm-kernel,
Linux Kernel Mailing List, linux-arm-msm, Tingwei Zhang,
Coresight ML
In-Reply-To: <7b69c9752713ce22f04688e83ec78f8aa67c63dc.1589558615.git.saiprakash.ranjan@codeaurora.org>
On Fri, 15 May 2020 at 10:23, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> From: Tingwei Zhang <tingwei@codeaurora.org>
>
> Add "qcom,skip-power-up" property to identify systems which can
> skip powering up of trace unit since they share the same power
> domain as their CPU core. This is required to identify such
> systems with hardware errata which stops the CPU watchdog counter
> when the power up bit is set (TRCPDCR.PU).
>
> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
> Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index 846f6daae71b..e4b2eda0b53b 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -108,6 +108,13 @@ its hardware characteristcs.
> * arm,cp14: must be present if the system accesses ETM/PTM management
> registers via co-processor 14.
>
> + * qcom,skip-power-up: boolean. Indicates that an implementation can
> + skip powering up the trace unit. TRCPDCR.PU does not have to be set
> + on Qualcomm Technologies Inc. systems since ETMs are in the same power
> + domain as their CPU cores. This property is required to identify such
> + systems with hardware errata where the CPU watchdog counter is stopped
> + when TRCPDCR.PU is set.
> +
> * Optional property for TMC:
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> * arm,buffer-size: size of contiguous buffer space for TMC ETR
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Mark Brown @ 2020-05-21 15:51 UTC (permalink / raw)
To: Serge Semin
Cc: Andy Shevchenko, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521121228.aqplh6eftylnys3p@mobilestation>
[-- Attachment #1: Type: text/plain, Size: 553 bytes --]
On Thu, May 21, 2020 at 03:12:28PM +0300, Serge Semin wrote:
> Well, for me both solutions are equal except mine consumes less stack memory.
> The only reason why your solution might be better is that if DW DMA driver or
> the DMA engine subsystem changed the dw_dma_slave structure instance passed to
> the dma_request_channel() method, which non of them do. So I'll leave this for
> Mark to decide. Mark, could you give us your final word about this?
Honestly I'm struggling to care either way. I guess saving a bit of
stack is potentially useful.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH V6 1/7] soc: qcom: geni: Support for ICC voting
From: Matthias Kaehlcke @ 2020-05-21 15:50 UTC (permalink / raw)
To: Akash Asthana
Cc: gregkh, agross, bjorn.andersson, wsa, broonie, mark.rutland,
robh+dt, linux-i2c, linux-spi, devicetree, swboyd, mgautam,
linux-arm-msm, linux-serial, dianders, msavaliy, evgreen
In-Reply-To: <1590049764-20912-2-git-send-email-akashast@codeaurora.org>
Hi Akash,
On Thu, May 21, 2020 at 01:59:18PM +0530, Akash Asthana wrote:
> Add necessary macros and structure variables to support ICC BW
> voting from individual SE drivers.
>
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in V2:
> - As per Bjorn's comment dropped enums for ICC paths, given the three
> paths individual members
>
> Changes in V3:
> - Add geni_icc_get, geni_icc_vote_on and geni_icc_vote_off as helper API.
> - Add geni_icc_path structure in common header
>
> Changes in V4:
> - As per Bjorn's comment print error message in geni_icc_get if return
> value is not -EPROBE_DEFER.
> - As per Bjorn's comment remove NULL on path before calling icc_set_bw
> API.
> - As per Bjorn's comment drop __func__ print.
> - As per Matthias's comment, make ICC path a array instead of individual
> member entry in geni_se struct.
>
> Changes in V5:
> - As per Matthias's comment defined enums for ICC paths.
> - Integrate icc_enable/disable with power on/off call for driver.
> - As per Matthias's comment added icc_path_names array to print icc path name
> in failure case.
> - As per Georgi's suggestion assume peak_bw = avg_bw if not mentioned.
>
> Changes in V6:
> - Addressed nitpicks from Matthias.
>
> Note: I have ignored below check patch suggestion because it was throwing
> compilation error as 'icc_ddr' is not compile time comstant.
>
> WARNING: char * array declaration might be better as static const
> - FILE: drivers/soc/qcom/qcom-geni-se.c:726:
> - const char *icc_names[] = {"qup-core", "qup-config", icc_ddr};
>
> drivers/soc/qcom/qcom-geni-se.c | 92 +++++++++++++++++++++++++++++++++++++++++
> include/linux/qcom-geni-se.h | 42 +++++++++++++++++++
> 2 files changed, 134 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 7d622ea..0b2526d 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -92,6 +92,9 @@ struct geni_wrapper {
> struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> };
>
> +static const char * const icc_path_names[] = {"qup-core", "qup-config",
> + "qup-memory"};
> +
> #define QUP_HW_VER_REG 0x4
>
> /* Common SE registers */
> @@ -720,6 +723,95 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len)
> }
> EXPORT_SYMBOL(geni_se_rx_dma_unprep);
>
> +int geni_icc_get(struct geni_se *se, const char *icc_ddr)
> +{
> + int i, err;
> + const char *icc_names[] = {"qup-core", "qup-config", icc_ddr};
> +
> + for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++) {
> + if (!icc_names[i])
> + continue;
> +
> + se->icc_paths[i].path = devm_of_icc_get(se->dev, icc_names[i]);
> + if (IS_ERR(se->icc_paths[i].path))
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + err = PTR_ERR(se->icc_paths[i].path);
> + if (err != -EPROBE_DEFER)
> + dev_err_ratelimited(se->dev, "Failed to get ICC path:%s :%d\n",
That's still an odd format, especially the colon before the error code. My
suggestion was "... path 'qup-core': 42" i.e. "... path '%s': %d".
I don't want to stall the series on nits though, if there is no need for
a respin for other reasons this can be also fixed with a patch after this
has landed.
I'm still not overly convinced about having two bandwidth values for what
might happen in the future (or not). Typically unused functions or struct
members that are added just in case tend to be rejected, since they can be
added when the need actually arises. Anyway, as long as maintainers are
happy with it I won't object.
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox