* Re: [RFC PATCH V4 1/4] media: v4l2-mem2mem: add v4l2_m2m_suspend, v4l2_m2m_resume
From: Jerry-ch Chen @ 2020-05-22 6:01 UTC (permalink / raw)
To: Tomasz Figa
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: <20200521171101.GA243874@chromium.org>
Hi Tomasz,
On Thu, 2020-05-21 at 17:11 +0000, Tomasz Figa wrote:
> 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>
>
Ok, I've added it in the commit message.
Thanks and Best regards,
Jerry
> [Corrected Hans's email address.]
> Hans, does this look good to you?
>
> Best regards,
> Tomasz
>
^ permalink raw reply
* Re: [PATCH v8 5/5] dt-bindings: arm: fsl: add different Protonic boards
From: Oleksij Rempel @ 2020-05-22 6:03 UTC (permalink / raw)
To: robh
Cc: Mark Rutland, devicetree, Shawn Guo, Sascha Hauer, linux-kernel,
Rob Herring, NXP Linux Team, Pengutronix Kernel Team,
Fabio Estevam, linux-arm-kernel
In-Reply-To: <20200521200002.GA2800876@bogus>
[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]
On Thu, May 21, 2020 at 02:00:02PM -0600, robh@kernel.org wrote:
> On Wed, 20 May 2020 17:41:16 +0200, Oleksij Rempel wrote:
> > Add Protonic PRTI6Q, WD2, RVT, VT7 boards.
> >
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > Documentation/devicetree/bindings/arm/fsl.yaml | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
>
>
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>
> If a tag was not added on purpose, please state why and what changed.
Sorry, there is no special reason. I just missed it.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings
From: Sean Anderson @ 2020-05-22 6:29 UTC (permalink / raw)
To: Anup Patel
Cc: Anup Patel, Palmer Dabbelt, Paul Walmsley, Albert Ou, Rob Herring,
Daniel Lezcano, Thomas Gleixner, devicetree, Damien Le Moal,
linux-kernel@vger.kernel.org List, Atish Patra, Alistair Francis,
linux-riscv
In-Reply-To: <CAAhSdy0OuxCwMVPBrvPpYMfVrhUuY3pONysk75yognOM5-0U+g@mail.gmail.com>
On 5/22/20 1:54 AM, Anup Patel wrote:
> On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> On 5/21/20 9:45 AM, Anup Patel wrote:
>>> +Required properties:
>>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
>>> + detailed implementation in case that specific bugs need to be worked around.
>>
>> Should the "riscv,clint0" compatible string be documented here? This
>
> Yes, I forgot to add this compatible string. I will add in v2.
>
>> peripheral is not really specific to sifive, as it is present in most
>> rocket-chip cores.
>
> I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
> FPGAs but this IP is only documented as part of SiFive FU540 SOC.
> (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
>
> The RISC-V foundation should host the CLINT spec independently
> under https://github.com/riscv and make CLINT spec totally open.
>
> For now, I have documented it just like PLIC DT bindings found at:
> Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
The PLIC seems to have its own RISC-V-sponsored documentation [1] which
was split off from the older privileged specs. By your logic above,
should it be renamed to riscv,plic0.txt (with a corresponding change in
the documented compatible strings)?
[1] https://github.com/riscv/riscv-plic-spec
>
> If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
>
> @Palmer ?? @Paul ??
>
> Regards,
> Anup
>
--Sean
^ permalink raw reply
* Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings
From: Anup Patel @ 2020-05-22 6:36 UTC (permalink / raw)
To: Sean Anderson
Cc: Anup Patel, Palmer Dabbelt, Paul Walmsley, Albert Ou, Rob Herring,
Daniel Lezcano, Thomas Gleixner, devicetree, Damien Le Moal,
linux-kernel@vger.kernel.org List, Atish Patra, Alistair Francis,
linux-riscv
In-Reply-To: <c0e9e625-daf8-b72f-2237-06018ff5d8a0@gmail.com>
On Fri, May 22, 2020 at 11:59 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 5/22/20 1:54 AM, Anup Patel wrote:
> > On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
> >>
> >> On 5/21/20 9:45 AM, Anup Patel wrote:
> >>> +Required properties:
> >>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> >>> + detailed implementation in case that specific bugs need to be worked around.
> >>
> >> Should the "riscv,clint0" compatible string be documented here? This
> >
> > Yes, I forgot to add this compatible string. I will add in v2.
> >
> >> peripheral is not really specific to sifive, as it is present in most
> >> rocket-chip cores.
> >
> > I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
> > FPGAs but this IP is only documented as part of SiFive FU540 SOC.
> > (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
> >
> > The RISC-V foundation should host the CLINT spec independently
> > under https://github.com/riscv and make CLINT spec totally open.
> >
> > For now, I have documented it just like PLIC DT bindings found at:
> > Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
>
> The PLIC seems to have its own RISC-V-sponsored documentation [1] which
> was split off from the older privileged specs. By your logic above,
> should it be renamed to riscv,plic0.txt (with a corresponding change in
> the documented compatible strings)?
>
> [1] https://github.com/riscv/riscv-plic-spec
For PLIC bindings, we can certainly do the renaming because now
we have PLIC v1 specification hosted on RISC-V Foundation Github.
Regards,
Anup
^ permalink raw reply
* Re: [PATCH v4 08/13] mips: Add CPS_NS16550_WIDTH config
From: Thomas Bogendoerfer @ 2020-05-22 7:29 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, devicetree,
Philippe Mathieu-Daudé, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, linux-mips, linux-kernel
In-Reply-To: <20200521140725.29571-9-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:19PM +0300, Serge Semin wrote:
> On some platforms IO-memory might require to use a proper load/store
> instructions (like Baikal-T1 IO-memory). To fix the cps-vec UART debug
> printout let's add the CONFIG_CPS_NS16550_WIDTH config to determine which
> instructions lb/sb, lh/sh or lw/sw are required for MMIO operations.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/Kconfig.debug | 10 ++++++++++
> arch/mips/kernel/cps-vec-ns16550.S | 18 ++++++++++++++++--
> 2 files changed, 26 insertions(+), 2 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 11/13] mips: Add udelay lpj numbers adjustment
From: Thomas Bogendoerfer @ 2020-05-22 7:29 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Jiaxun Yang, Paul Burton,
Ralf Baechle, Arnd Bergmann, Rob Herring, devicetree,
Allison Randal, Thomas Gleixner, linux-mips, linux-kernel
In-Reply-To: <20200521140725.29571-12-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:22PM +0300, Serge Semin wrote:
> Loops-per-jiffies is a special number which represents a number of
> noop-loop cycles per CPU-scheduler quantum - jiffies. As you
> understand aside from CPU-specific implementation it depends on
> the CPU frequency. So when a platform has the CPU frequency fixed,
> we have no problem and the current udelay interface will work
> just fine. But as soon as CPU-freq driver is enabled and the cores
> frequency changes, we'll end up with distorted udelay's. In order
> to fix this we have to accordinly adjust the per-CPU udelay_val
> (the same as the global loops_per_jiffy) number. This can be done
> in the CPU-freq transition event handler. We subscribe to that event
> in the MIPS arch time-inititalization method.
>
> Co-developed-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/kernel/time.c | 70 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 06/13] mips: Add CP0 Write Merge config support
From: Thomas Bogendoerfer @ 2020-05-22 7:28 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, devicetree, Jiaxun Yang,
Philippe Mathieu-Daudé, Huacai Chen, Paul Cercueil,
Masahiro Yamada, Zhou Yanjie, WANG Xuerui,
周琰杰 (Zhou Yanjie), YunQiang Su,
Liangliang Huang, Thomas Gleixner, linux-mips, linux-kernel
In-Reply-To: <20200521140725.29571-7-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:17PM +0300, Serge Semin wrote:
> CP0 config register may indicate whether write-through merging
> is allowed. Currently there are two types of the merging available:
> SysAD Valid and Full modes. Whether each of them are supported by
> the core is implementation dependent. Moreover whether the ability
> to change the mode also depends on the chip family instance. Taking
> into account all of this we created a dedicated mm_config() method
> to detect and enable merging if it's supported. It is called for
> MIPS-type processors at CPU-probe stage and attempts to detect whether
> the write merging is available. If it's known to be supported and
> switchable, then switch on the full mode. Otherwise just perform the
> CP0.Config.MM field analysis.
>
> In addition there are platforms like InterAptiv/ProAptiv, which do have
> the MM bit field set by default, but having write-through cacheing
> unsupported makes write-merging also unsupported. In this case we just
> ignore the MM field value.
>
> Co-developed-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/include/asm/cpu-features.h | 8 +++++
> arch/mips/include/asm/cpu.h | 4 ++-
> arch/mips/include/asm/mipsregs.h | 3 ++
> arch/mips/kernel/cpu-probe.c | 48 ++++++++++++++++++++++++++++
> 4 files changed, 62 insertions(+), 1 deletion(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 03/13] mips: Add MIPS Release 5 support
From: Thomas Bogendoerfer @ 2020-05-22 7:27 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, devicetree, Jiaxun Yang,
Alexander Lobakin, Huacai Chen, Nathan Chancellor, Ard Biesheuvel,
Cedric Hombourger, Thomas Gleixner, Ingo Molnar,
Sebastian Andrzej Siewior, Philippe Mathieu-Daudé,
Guenter Roeck, Paul Cercueil, Zhou Yanjie, Masahiro Yamada,
Greg Kroah-Hartman, Allison Randal, Liangliang Huang,
周琰杰 (Zhou Yanjie), YunQiang Su, Zou Wei,
Oleksij Rempel, Kamal Dasu, linux-mips, linux-kernel, kvm
In-Reply-To: <20200521140725.29571-4-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:14PM +0300, Serge Semin wrote:
> There are five MIPS32/64 architecture releases currently available:
> from 1 to 6 except fourth one, which was intentionally skipped.
> Three of them can be called as major: 1st, 2nd and 6th, that not only
> have some system level alterations, but also introduced significant
> core/ISA level updates. The rest of the MIPS architecture releases are
> minor.
>
> Even though they don't have as much ISA/system/core level changes
> as the major ones with respect to the previous releases, they still
> provide a set of updates (I'd say they were intended to be the
> intermediate releases before a major one) that might be useful for the
> kernel and user-level code, when activated by the kernel or compiler.
> In particular the following features were introduced or ended up being
> available at/after MIPS32/64 Release 5 architecture:
> + the last release of the misaligned memory access instructions,
> + virtualisation - VZ ASE - is optional component of the arch,
> + SIMD - MSA ASE - is optional component of the arch,
> + DSP ASE is optional component of the arch,
> + CP0.Status.FR=1 for CP1.FIR.F64=1 (pure 64-bit FPU general registers)
> must be available if FPU is implemented,
> + CP1.FIR.Has2008 support is required so CP1.FCSR.{ABS2008,NAN2008} bits
> are available.
> + UFR/UNFR aliases to access CP0.Status.FR from user-space by means of
> ctc1/cfc1 instructions (enabled by CP0.Config5.UFR),
> + CP0.COnfig5.LLB=1 and eretnc instruction are implemented to without
> accidentally clearing LL-bit when returning from an interrupt,
> exception, or error trap,
> + XPA feature together with extended versions of CPx registers is
> introduced, which needs to have mfhc0/mthc0 instructions available.
>
> So due to these changes GNU GCC provides an extended instructions set
> support for MIPS32/64 Release 5 by default like eretnc/mfhc0/mthc0. Even
> though the architecture alteration isn't that big, it still worth to be
> taken into account by the kernel software. Finally we can't deny that
> some optimization/limitations might be found in future and implemented
> on some level in kernel or compiler. In this case having even
> intermediate MIPS architecture releases support would be more than
> useful.
>
> So the most of the changes provided by this commit can be split into
> either compile- or runtime configs related. The compile-time related
> changes are caused by adding the new CONFIG_CPU_MIPS32_R5/CONFIG_CPU_MIPSR5
> configs and concern the code activating MIPSR2 or MIPSR6 already
> implemented features (like eretnc/LLbit, mthc0/mfhc0). In addition
> CPU_HAS_MSA can be now freely enabled for MIPS32/64 release 5 based
> platforms as this is done for CPU_MIPS32_R6 CPUs. The runtime changes
> concerns the features which are handled with respect to the MIPS ISA
> revision detected at run-time by means of CP0.Config.{AT,AR} bits. Alas
> these fields can be used to detect either r1 or r2 or r6 releases.
> But since we know which CPUs in fact support the R5 arch, we can manually
> set MIPS_CPU_ISA_M32R5/MIPS_CPU_ISA_M64R5 bit of c->isa_level and then
> use cpu_has_mips32r5/cpu_has_mips64r5 where it's appropriate.
>
> Since XPA/EVA provide too complex alterationss and to have them used with
> MIPS32 Release 2 charged kernels (for compatibility with current platform
> configs) they are left to be setup as a separate kernel configs.
>
> Co-developed-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/Kconfig | 56 +++++++++++++++++++++++++---
> arch/mips/Makefile | 2 +
> arch/mips/include/asm/asmmacro.h | 18 +++++----
> arch/mips/include/asm/compiler.h | 5 +++
> arch/mips/include/asm/cpu-features.h | 27 ++++++++++----
> arch/mips/include/asm/cpu-info.h | 2 +-
> arch/mips/include/asm/cpu-type.h | 7 +++-
> arch/mips/include/asm/cpu.h | 10 +++--
> arch/mips/include/asm/fpu.h | 4 +-
> arch/mips/include/asm/hazards.h | 8 ++--
> arch/mips/include/asm/module.h | 4 ++
> arch/mips/include/asm/stackframe.h | 2 +-
> arch/mips/include/asm/switch_to.h | 8 ++--
> arch/mips/kernel/cpu-probe.c | 17 +++++++++
> arch/mips/kernel/entry.S | 6 +--
> arch/mips/kernel/proc.c | 4 ++
> arch/mips/kernel/r4k_fpu.S | 14 +++----
> arch/mips/kvm/vz.c | 6 +--
> arch/mips/lib/csum_partial.S | 6 ++-
> arch/mips/mm/c-r4k.c | 7 ++--
> arch/mips/mm/sc-mips.c | 7 ++--
> 21 files changed, 163 insertions(+), 57 deletions(-)
applied to mips-next. I've changed the two /* fall through */ by fallthrough;
while appliny. Running checkpatch would have caught that ;-)
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 12/13] mips: csrc-r4k: Mark R4K timer as unstable if CPU freq changes
From: Thomas Bogendoerfer @ 2020-05-22 7:29 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, devicetree,
Jiaxun Yang, Alexander Lobakin, Huacai Chen, Vincenzo Frascino,
Thomas Gleixner, linux-mips, linux-kernel
In-Reply-To: <20200521140725.29571-13-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:23PM +0300, Serge Semin wrote:
> Commit 07d69579e7fe ("MIPS: Don't register r4k sched clock when CPUFREQ
> enabled") disabled the r4k-clock usage for scheduler ticks counting due
> to the scheduler being non-tolerant for unstable clocks sources. For the
> same reason the clock should be used in the system clocksource framework
> with care. As soon as CPU frequency changes the clocksource framework
> should be notified about this by marking the R4K timer being unstable
> (which it really is, since the ticks rate has been changed synchronously
> with the CPU frequency).
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/Kconfig | 1 +
> arch/mips/kernel/csrc-r4k.c | 40 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 04/13] mips: Add MIPS Warrior P5600 support
From: Thomas Bogendoerfer @ 2020-05-22 7:28 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, devicetree, Jiaxun Yang, Huacai Chen,
Alexander Lobakin, Fangrui Song, Ard Biesheuvel,
Nathan Chancellor, Cedric Hombourger, linux-mips, linux-kernel
In-Reply-To: <20200521140725.29571-5-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:15PM +0300, Serge Semin wrote:
> This is a MIPS32 Release 5 based IP core with XPA, EVA, dual/quad issue
> exec pipes, MMU with two-levels TLB, UCA, MSA, MDU core level features
> and system level features like up to six P5600 calculation cores, CM2
> with L2 cache, IOCU/IOMMU (though might be unused depending on the
> system-specific IP core configuration), GIC, CPC, virtualisation module,
> eJTAG and PDtrace.
>
> As being MIPS32 Release 5 based core it provides all the features
> available by the CPU_MIPS32_R5 config, while adding a few more like
> UCA attribute support, availability of CPU-freq (by means of L2/CM
> clock ratio setting), EI/VI GIC modes detection at runtime.
>
> In addition to this if P5600 architecture is enabled modern GNU GCC
> provides a specific tuning for P5600 processors with respect to the
> classic MIPS32 Release 5. First of all branch-likely avoidance is
> activated only when the code is compiled with the speed optimization
> (avoidance is always enabled for the pure MIPS32 Release 5
> architecture). Secondly the madd/msub avoidance is enabled since
> madd/msub utilization isn't profitable due to overhead of getting the
> result out of the HI/LO registers. Multiply-accumulate instructions are
> activated and utilized together with the necessary code reorder when
> multiply-add/multiply-subtract statements are met. Finally load/store
> bonding is activated by default. All of these optimizations may make
> the code relatively faster than if just MIP32 release 5 architecture
> was requested.
>
> Co-developed-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/Kconfig | 37 +++++++++++++++++++++++++++++-----
> arch/mips/Makefile | 1 +
> arch/mips/include/asm/module.h | 2 ++
> 3 files changed, 35 insertions(+), 5 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 07/13] mips: Add CONFIG/CONFIG6/Cause reg fields macro
From: Thomas Bogendoerfer @ 2020-05-22 7:28 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, devicetree, Zhou Yanjie, Jiaxun Yang,
WANG Xuerui, Allison Randal, Greg Kroah-Hartman, Thomas Gleixner,
linux-mips, linux-kernel
In-Reply-To: <20200521140725.29571-8-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:18PM +0300, Serge Semin wrote:
> There are bit fields which persist in the MIPS CONFIG and CONFIG6
> registers, but haven't been described in the generic mipsregs.h
> header so far. In particular, the generic CONFIG bitfields are
> BE - endian mode, BM - burst mode, SB - SimpleBE, OCP interface mode
> indicator, UDI - user-defined "CorExtend" instructions, DSP - data
> scratch pad RAM present, ISP - instruction scratch pad RAM present,
> etc. The core-specific CONFIG6 bitfields are JRCD - jump register
> cache prediction disable, R6 - MIPSr6 extensions enable, IFUPerfCtl -
> IFU performance control, SPCD - sleep state performance counter, DLSB -
> disable load/store bonding. A new exception code reported in the
> ExcCode field of the Cause register: 30 - Parity/ECC error exception
> happened on either fetch, load or cache refill. Lets add them to the
> mipsregs.h header to be used in future platform code, which have them
> utilized.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/include/asm/mipsregs.h | 19 +++++++++++++++++++
> arch/mips/kernel/spram.c | 4 ++--
> 2 files changed, 21 insertions(+), 2 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 05/13] mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs
From: Thomas Bogendoerfer @ 2020-05-22 7:28 UTC (permalink / raw)
To: Serge Semin
Cc: Paul Burton, Serge Semin, Alexey Malahov, Ralf Baechle,
Arnd Bergmann, Rob Herring, devicetree, Jiaxun Yang, linux-mips,
linux-kernel
In-Reply-To: <20200521140725.29571-6-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:16PM +0300, Serge Semin wrote:
> Commit 1aeba347b3a9 ("MIPS: Hardcode cpu_has_mips* where target ISA
> allows") updated the cpu_has_mips* macro to be replaced with a constant
> expression where it's possible. By mistake it wasn't done correctly
> for cpu_has_mips64r1/cpu_has_mips64r2 macro. They are defined to
> be replaced with conditional expression __isa_range_or_flag(), which
> means either ISA revision being within the range or the corresponding
> CPU options flag was set at the probe stage or both being true at the
> same time. But the ISA level value doesn't indicate whether the ISA is
> MIPS32 or MIPS64. Due to this if we select MIPS32r1 - MIPS32r5
> architectures the __isa_range() macro will activate the
> cpu_has_mips64rX flags, which is incorrect. In order to fix the
> problem we make sure the 64bits CPU support is enabled by means of
> checking the flag cpu_has_64bits aside with proper ISA range and specific
> Revision flag being set.
>
> Fixes: 1aeba347b3a9 ("MIPS: Hardcode cpu_has_mips* where target ISA allows")
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/include/asm/cpu-features.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v4 13/13] mips: cevt-r4k: Update the r4k-clockevent frequency in sync with CPU
From: Thomas Bogendoerfer @ 2020-05-22 7:30 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, devicetree,
afzal mohammed, linux-mips, linux-kernel
In-Reply-To: <20200521140725.29571-14-Sergey.Semin@baikalelectronics.ru>
On Thu, May 21, 2020 at 05:07:24PM +0300, Serge Semin wrote:
> Due to being embedded into the CPU cores MIPS count/compare timer
> frequency is changed together with the CPU clocks alteration.
> In case if frequency really changes the kernel clockevent framework
> must be notified, otherwise the kernel timers won't work correctly.
> Fix this by calling clockevents_update_freq() for each r4k clockevent
> handlers registered per available CPUs.
>
> Traditionally MIPS r4k-clock are clocked with CPU frequency divided by 2.
> But this isn't true for some of the platforms. Due to this we have to save
> the basic CPU frequency, so then use it to scale the initial timer
> frequency (mips_hpt_frequency) and pass the updated value further to the
> clockevent framework.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/kernel/cevt-r4k.c | 44 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH] dt-bindings: mfd: mediatek: Add MT6397 Pin Controller
From: Lee Jones @ 2020-05-22 7:37 UTC (permalink / raw)
To: Matthias Brugger
Cc: matthias.bgg, Rob Herring, Mark Rutland, devicetree,
linux-arm-kernel, linux-kernel, linux-mediatek
In-Reply-To: <ce3747e5-41db-a168-0602-37337d8530f2@gmail.com>
On Thu, 21 May 2020, Matthias Brugger wrote:
> Hi Lee,
>
> On 15/01/2020 16:19, Rob Herring wrote:
> > On Fri, 10 Jan 2020 15:59:51 +0100, matthias.bgg@kernel.org wrote:
> >> From: Matthias Brugger <matthias.bgg@gmail.com>
> >>
> >> The MT6397 mfd includes a pin controller. Add binding
> >> a description for it.
> >>
> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> >>
> >> ---
> >>
> >> Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >
> > Acked-by: Rob Herring <robh@kernel.org>
> >
>
> It looks like this fall through the cracks.
> Would you consider to queue it or do you have further comments?
There is a current issue where Rob and I are both taking patches.
Sometimes we both assume the other will take the patch.
Apologies.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] dt-bindings: mfd: mediatek: Add MT6397 Pin Controller
From: Lee Jones @ 2020-05-22 7:37 UTC (permalink / raw)
To: matthias.bgg
Cc: Matthias Brugger, Mark Rutland, Rob Herring, devicetree,
linux-arm-kernel, linux-kernel, linux-mediatek
In-Reply-To: <20200110145952.9720-1-matthias.bgg@kernel.org>
On Fri, 10 Jan 2020, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <matthias.bgg@gmail.com>
>
> The MT6397 mfd includes a pin controller. Add binding
> a description for it.
>
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>
> ---
>
> Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +++++
> 1 file changed, 5 insertions(+)
Applied, thanks.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH v1 0/2] pwm: intel: Add PWM driver for new SoC
From: Rahul Tanwar @ 2020-05-22 7:41 UTC (permalink / raw)
To: thierry.reding, u.kleine-koenig, p.zabel, linux-pwm, robh+dt
Cc: linux-kernel, devicetree, andriy.shevchenko, songjun.Wu,
cheol.yong.kim, qi-ming.wu, Rahul Tanwar
Patch 1 adds dt binding document in YAML format.
Patch 2 add PWM driver for LGM SoC.
Rahul Tanwar (2):
Add YAML schema for a new PWM driver
Add PWM driver for LGM
.../devicetree/bindings/pwm/pwm-intel-lgm.yaml | 43 +++
drivers/pwm/Kconfig | 9 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-intel-lgm.c | 356 +++++++++++++++++++++
4 files changed, 409 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-intel-lgm.yaml
create mode 100644 drivers/pwm/pwm-intel-lgm.c
--
2.11.0
^ permalink raw reply
* [PATCH v1 1/2] Add YAML schema for a new PWM driver
From: Rahul Tanwar @ 2020-05-22 7:41 UTC (permalink / raw)
To: thierry.reding, u.kleine-koenig, p.zabel, linux-pwm, robh+dt
Cc: linux-kernel, devicetree, andriy.shevchenko, songjun.Wu,
cheol.yong.kim, qi-ming.wu, Rahul Tanwar
In-Reply-To: <cover.1590132733.git.rahul.tanwar@linux.intel.com>
Add DT bindings YAML schema for PWM controller driver of
Lightning Mountain(LGM) SoC.
Signed-off-by: Rahul Tanwar <rahul.tanwar@linux.intel.com>
---
.../devicetree/bindings/pwm/pwm-intel-lgm.yaml | 43 ++++++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-intel-lgm.yaml
diff --git a/Documentation/devicetree/bindings/pwm/pwm-intel-lgm.yaml b/Documentation/devicetree/bindings/pwm/pwm-intel-lgm.yaml
new file mode 100644
index 000000000000..adb33265aa5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-intel-lgm.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/pwm-intel-lgm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LGM SoC PWM controller
+
+maintainers:
+ - Rahul Tanwar <rahul.tanwar@intel.com>
+
+properties:
+ compatible:
+ const: intel,lgm-pwm
+
+ reg:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 2
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - "#pwm-cells"
+ - clocks
+ - resets
+
+examples:
+ - |
+ pwm: pwm@e0d00000 {
+ compatible = "intel,lgm-pwm";
+ reg = <0xe0d00000 0x30>;
+ #pwm-cells = <2>;
+ clocks = <&cgu0 126>;
+ resets = <&rcu0 0x30 21>;
+ };
--
2.11.0
^ permalink raw reply related
* [PATCH v1 2/2] Add PWM driver for LGM
From: Rahul Tanwar @ 2020-05-22 7:41 UTC (permalink / raw)
To: thierry.reding, u.kleine-koenig, p.zabel, linux-pwm, robh+dt
Cc: linux-kernel, devicetree, andriy.shevchenko, songjun.Wu,
cheol.yong.kim, qi-ming.wu, Rahul Tanwar
In-Reply-To: <cover.1590132733.git.rahul.tanwar@linux.intel.com>
Add PWM controller driver for Intel's Lightning Mountain(LGM) SoC.
Signed-off-by: Rahul Tanwar <rahul.tanwar@linux.intel.com>
---
drivers/pwm/Kconfig | 9 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-intel-lgm.c | 356 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 366 insertions(+)
create mode 100644 drivers/pwm/pwm-intel-lgm.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index eebbc917ac97..a582214f50b2 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -232,6 +232,15 @@ config PWM_IMX_TPM
To compile this driver as a module, choose M here: the module
will be called pwm-imx-tpm.
+config PWM_INTEL_LGM
+ tristate "Intel LGM PWM support"
+ depends on X86 || COMPILE_TEST
+ help
+ Generic PWM framework driver for LGM SoC.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-intel-lgm.
+
config PWM_JZ4740
tristate "Ingenic JZ47xx PWM support"
depends on MACH_INGENIC
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 9a475073dafc..c16a972a101d 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_PWM_IMG) += pwm-img.o
obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o
obj-$(CONFIG_PWM_IMX27) += pwm-imx27.o
obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o
+obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o
obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o
obj-$(CONFIG_PWM_LP3943) += pwm-lp3943.o
obj-$(CONFIG_PWM_LPC18XX_SCT) += pwm-lpc18xx-sct.o
diff --git a/drivers/pwm/pwm-intel-lgm.c b/drivers/pwm/pwm-intel-lgm.c
new file mode 100644
index 000000000000..e307fd2457df
--- /dev/null
+++ b/drivers/pwm/pwm-intel-lgm.c
@@ -0,0 +1,356 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Intel Corporation.
+ */
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#define PWM_FAN_CON0 0x0
+#define PWM_FAN_EN_EN BIT(0)
+#define PWM_FAN_EN_DIS 0x0
+#define PWM_FAN_EN_MSK BIT(0)
+#define PWM_FAN_MODE_2WIRE 0x0
+#define PWM_FAN_MODE_4WIRE 0x1
+#define PWM_FAN_MODE_MSK BIT(1)
+#define PWM_FAN_PWM_DIS_DIS 0x0
+#define PWM_FAN_PWM_DIS_MSK BIT(2)
+#define PWM_TACH_EN_EN 0x1
+#define PWM_TACH_EN_MSK BIT(4)
+#define PWM_TACH_PLUS_2 0x0
+#define PWM_TACH_PLUS_4 0x1
+#define PWM_TACH_PLUS_MSK BIT(5)
+#define PWM_FAN_DC_MSK GENMASK(23, 16)
+
+#define PWM_FAN_CON1 0x4
+#define PWM_FAN_MAX_RPM_MSK GENMASK(15, 0)
+
+#define PWM_FAN_STAT 0x10
+#define PWM_FAN_TACH_MASK GENMASK(15, 0)
+
+#define MAX_RPM (BIT(16) - 1)
+#define DFAULT_RPM 4000
+#define MAX_DUTY_CYCLE (BIT(8) - 1)
+
+#define FRAC_BITS 10
+#define TWO_TENTH 204
+
+#define TWO_SECONDS 2000
+#define IGNORE_FIRST_ERR 1
+#define THIRTY_SECS_WINDOW 15
+#define ERR_CNT_THRESHOLD 6
+
+struct intel_pwm_chip {
+ struct pwm_chip chip;
+ struct regmap *regmap;
+ struct clk *clk;
+ struct reset_control *rst;
+ u32 tach_en;
+ u32 max_rpm;
+ u32 set_rpm;
+ u32 set_dc;
+ struct delayed_work work;
+};
+
+static inline struct intel_pwm_chip *to_intel_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct intel_pwm_chip, chip);
+}
+
+static int pwm_update_dc(struct intel_pwm_chip *pc, u32 val)
+{
+ return regmap_update_bits(pc->regmap, PWM_FAN_CON0, PWM_FAN_DC_MSK,
+ FIELD_PREP(PWM_FAN_DC_MSK, val));
+}
+
+static int intel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+ int duty_ns, int period_ns)
+{
+ struct intel_pwm_chip *pc = to_intel_pwm_chip(chip);
+ u32 val;
+
+ val = DIV_ROUND_CLOSEST(duty_ns * MAX_DUTY_CYCLE, period_ns);
+ val = min_t(u32, val, MAX_DUTY_CYCLE);
+
+ if (pc->tach_en) {
+ pc->set_dc = val;
+ pc->set_rpm = val * pc->max_rpm / MAX_DUTY_CYCLE;
+ }
+
+ return pwm_update_dc(pc, val);
+}
+
+static int intel_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct intel_pwm_chip *pc = to_intel_pwm_chip(chip);
+ struct regmap *regmap = pc->regmap;
+
+ regmap_update_bits(regmap, PWM_FAN_CON0,
+ PWM_FAN_EN_MSK, PWM_FAN_EN_EN);
+
+ if (pc->tach_en)
+ schedule_delayed_work(&pc->work, msecs_to_jiffies(10000));
+
+ return 0;
+}
+
+static void intel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct intel_pwm_chip *pc = to_intel_pwm_chip(chip);
+ struct regmap *regmap = pc->regmap;
+
+ if (pc->tach_en)
+ cancel_delayed_work_sync(&pc->work);
+
+ regmap_update_bits(regmap, PWM_FAN_CON0,
+ PWM_FAN_EN_MSK, PWM_FAN_EN_DIS);
+}
+
+static const struct pwm_ops intel_pwm_ops = {
+ .config = intel_pwm_config,
+ .enable = intel_pwm_enable,
+ .disable = intel_pwm_disable,
+ .owner = THIS_MODULE,
+};
+
+static void tach_work(struct work_struct *work)
+{
+ struct intel_pwm_chip *pc = container_of(work, struct intel_pwm_chip,
+ work.work);
+ struct regmap *regmap = pc->regmap;
+ u32 fan_tach, fan_dc, val;
+ s32 diff;
+ static u32 fanspeed_err_cnt, time_window, delta_dc;
+
+ /*
+ * Fan speed is tracked by reading the active duty cycle of PWM output
+ * from the active duty cycle register. Some variance in the duty cycle
+ * register value is expected. So we set a time window of 30 seconds and
+ * if we detect inaccurate fan speed 6 times within 30 seconds then we
+ * mark it as fan speed problem and fix it by readjusting the duty cycle.
+ */
+
+ if (fanspeed_err_cnt > IGNORE_FIRST_ERR)
+ /*
+ * Ignore first time we detect inaccurate fan speed
+ * because it is expected during bootup.
+ */
+ time_window++;
+
+ if (time_window == THIRTY_SECS_WINDOW) {
+ /*
+ * This work is scheduled every 2 seconds i.e. each time_window
+ * counter step roughly mean 2 seconds. When the time window
+ * reaches 30 seconds, reset all the counters/logic.
+ */
+ fanspeed_err_cnt = 0;
+ delta_dc = 0;
+ time_window = 0;
+ }
+
+ regmap_read(regmap, PWM_FAN_STAT, &fan_tach);
+ fan_tach &= PWM_FAN_TACH_MASK;
+ if (!fan_tach)
+ goto restart_work;
+
+ val = DIV_ROUND_CLOSEST(pc->set_rpm << FRAC_BITS, fan_tach);
+ diff = val - BIT(FRAC_BITS);
+
+ if (abs(diff) > TWO_TENTH) {
+ /* if duty cycle diff is more than two tenth, detect it as error */
+ if (fanspeed_err_cnt > IGNORE_FIRST_ERR)
+ delta_dc += val;
+ fanspeed_err_cnt++;
+ }
+
+ if (fanspeed_err_cnt == ERR_CNT_THRESHOLD) {
+ /*
+ * We detected fan speed errors 6 times with 30 seconds.
+ * Fix the error by readjusting duty cycle and reset
+ * our counters/logic.
+ */
+ fan_dc = pc->set_dc * delta_dc >> (FRAC_BITS + 2);
+ fan_dc = min_t(u32, fan_dc, MAX_DUTY_CYCLE);
+ pwm_update_dc(pc, fan_dc);
+ fanspeed_err_cnt = 0;
+ delta_dc = 0;
+ time_window = 0;
+ }
+
+restart_work:
+ /*
+ * Fan speed doesn't need continous tracking. Schedule this work
+ * every two seconds so it doesn't steal too much cpu cycles.
+ */
+ schedule_delayed_work(&pc->work, msecs_to_jiffies(TWO_SECONDS));
+}
+
+static void pwm_init(struct intel_pwm_chip *pc)
+{
+ struct device *dev = pc->chip.dev;
+ struct regmap *regmap = pc->regmap;
+ u32 max_rpm, fan_wire, tach_plus, con0_val, con0_mask;
+
+ if (device_property_read_u32(dev, "intel,fan-wire", &fan_wire))
+ fan_wire = 2; /* default is 2 wire mode */
+
+ con0_val = FIELD_PREP(PWM_FAN_PWM_DIS_MSK, PWM_FAN_PWM_DIS_DIS);
+ con0_mask = PWM_FAN_PWM_DIS_MSK | PWM_FAN_MODE_MSK;
+
+ switch (fan_wire) {
+ case 2 ... 3:
+ con0_val |= FIELD_PREP(PWM_FAN_MODE_MSK, PWM_FAN_MODE_2WIRE);
+ break;
+ case 4:
+ con0_val |= FIELD_PREP(PWM_FAN_MODE_MSK, PWM_FAN_MODE_4WIRE) |
+ FIELD_PREP(PWM_TACH_EN_MSK, PWM_TACH_EN_EN);
+ con0_mask |= PWM_TACH_EN_MSK | PWM_TACH_PLUS_MSK;
+ pc->tach_en = 1;
+ break;
+ default:
+ /* default is 2wire mode */
+ con0_val |= FIELD_PREP(PWM_FAN_MODE_MSK, PWM_FAN_MODE_2WIRE);
+ break;
+ }
+
+ if (pc->tach_en) {
+ if (device_property_read_u32(dev, "intel,tach-plus",
+ &tach_plus))
+ tach_plus = 2;
+
+ switch (tach_plus) {
+ case 2:
+ con0_val |= FIELD_PREP(PWM_TACH_PLUS_MSK,
+ PWM_TACH_PLUS_2);
+ break;
+ case 4:
+ con0_val |= FIELD_PREP(PWM_TACH_PLUS_MSK,
+ PWM_TACH_PLUS_4);
+ break;
+ default:
+ con0_val |= FIELD_PREP(PWM_TACH_PLUS_MSK,
+ PWM_TACH_PLUS_2);
+ break;
+ }
+
+ if (device_property_read_u32(dev, "intel,max-rpm", &max_rpm))
+ max_rpm = DFAULT_RPM;
+
+ max_rpm = min_t(u32, max_rpm, MAX_RPM);
+ if (max_rpm == 0)
+ max_rpm = DFAULT_RPM;
+
+ pc->max_rpm = max_rpm;
+ INIT_DEFERRABLE_WORK(&pc->work, tach_work);
+ regmap_update_bits(regmap, PWM_FAN_CON1,
+ PWM_FAN_MAX_RPM_MSK, max_rpm);
+ }
+
+ regmap_update_bits(regmap, PWM_FAN_CON0, con0_mask, con0_val);
+}
+
+static const struct regmap_config pwm_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+};
+
+static int intel_pwm_probe(struct platform_device *pdev)
+{
+ struct intel_pwm_chip *pc;
+ struct device *dev = &pdev->dev;
+ void __iomem *io_base;
+ int ret;
+
+ pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
+ if (!pc)
+ return -ENOMEM;
+
+ io_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(io_base))
+ return PTR_ERR(io_base);
+
+ pc->regmap = devm_regmap_init_mmio(dev, io_base, &pwm_regmap_config);
+ if (IS_ERR(pc->regmap)) {
+ ret = PTR_ERR(pc->regmap);
+ dev_err(dev, "failed to init register map: %d\n", ret);
+ return ret;
+ }
+
+ pc->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(pc->clk)) {
+ ret = PTR_ERR(pc->clk);
+ dev_err(dev, "failed to get clock: %d\n", ret);
+ return ret;
+ }
+
+ pc->rst = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(pc->rst)) {
+ ret = PTR_ERR(pc->rst);
+ dev_err(dev, "failed to get reset control: %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(pc->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable clock\n");
+ return ret;
+ }
+
+ reset_control_deassert(pc->rst);
+
+ pc->chip.dev = dev;
+ pc->chip.ops = &intel_pwm_ops;
+ pc->chip.npwm = 1;
+
+ pwm_init(pc);
+
+ ret = pwmchip_add(&pc->chip);
+ if (ret < 0) {
+ dev_err(dev, "failed to add PWM chip: %d\n", ret);
+ clk_disable_unprepare(pc->clk);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, pc);
+ return 0;
+}
+
+static int intel_pwm_remove(struct platform_device *pdev)
+{
+ struct intel_pwm_chip *pc = platform_get_drvdata(pdev);
+ int ret;
+
+ if (pc->tach_en)
+ cancel_delayed_work_sync(&pc->work);
+
+ ret = pwmchip_remove(&pc->chip);
+ if (ret < 0)
+ return ret;
+
+ reset_control_assert(pc->rst);
+
+ clk_disable_unprepare(pc->clk);
+
+ return 0;
+}
+
+static const struct of_device_id intel_pwm_of_match[] = {
+ { .compatible = "intel,lgm-pwm" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, intel_pwm_of_match);
+
+static struct platform_driver intel_pwm_driver = {
+ .driver = {
+ .name = "intel-pwm",
+ .of_match_table = intel_pwm_of_match,
+ },
+ .probe = intel_pwm_probe,
+ .remove = intel_pwm_remove,
+};
+module_platform_driver(intel_pwm_driver);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v4 06/11] net: ethernet: mtk-eth-mac: new driver
From: Bartosz Golaszewski @ 2020-05-22 7:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Stephane Le Provost, Pedro Tsai, Andrew Perepech, Rob Herring,
David S . Miller, Matthias Brugger, John Crispin, Sean Wang,
Mark Lee, Jakub Kicinski, Fabien Parent, Heiner Kallweit,
Edwin Peer, DTML, linux-kernel@vger.kernel.org, Networking,
Linux ARM, moderated list:ARM/Mediatek SoC...,
Bartosz Golaszewski
In-Reply-To: <CAK8P3a1nhPj6kRhwyXzDK3BGbh66XG6Fmp44QuM1NhFPPBTtPQ@mail.gmail.com>
śr., 20 maj 2020 o 23:23 Arnd Bergmann <arnd@arndb.de> napisał(a):
>
> On Wed, May 20, 2020 at 7:35 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > śr., 20 maj 2020 o 16:37 Arnd Bergmann <arnd@arndb.de> napisał(a):
>
> > > I just noticed how the naming of NET_MEDIATEK_MAC and NET_MEDIATEK_SOC
> > > for two different drivers doing the same thing is really confusing.
> > >
> > > Maybe someone can come up with a better name, such as one
> > > based on the soc it first showed up in.
> > >
> >
> > This has been discussed under one of the previous submissions.
> > MediaTek wants to use this IP on future designs as well and it's
> > already used on multiple SoCs so they want the name to be generic. I
> > also argued that this is a driver strongly tied to a specific
> > platform(s) so if someone wants to compile it - they probably know
> > what they're doing.
> >
> > That being said: I verified with MediaTek and the name of the IP I can
> > use is "star" so they proposed "mtk-star-eth". I would personally
> > maybe go with "mtk-star-mac". How about those two?
>
> Both seem fine to me. If this was previously discussed, I don't want
> do further bike-shedding and I'd trust you to pick a sensible name
> based on the earlier discussions.
>
> > > + /* One of the counters reached 0x8000000 - update stats and
> > > > + * reset all counters.
> > > > + */
> > > > + if (unlikely(status & MTK_MAC_REG_INT_STS_MIB_CNT_TH)) {
> > > > + mtk_mac_intr_disable_stats(priv);
> > > > + schedule_work(&priv->stats_work);
> > > > + }
> > > > + befor
> > > > + mtk_mac_intr_ack_all(priv);
> > >
> > > The ack here needs to be dropped, otherwise you can get further
> > > interrupts before the bottom half has had a chance to run.
> > >
> >
> > My thinking was this: if I mask the relevant interrupt (TX/RX
> > complete) and ack it right away, the status bit will be asserted on
> > the next packet received/sent but the process won't get interrupted
> > and when I unmask it, it will fire right away and I won't have to
> > recheck the status register. I noticed that if I ack it at the end of
> > napi poll callback, I end up missing certain TX complete interrupts
> > and end up seeing a lot of retransmissions even if I reread the status
> > register. I'm not yet sure where this race happens.
>
> Right, I see. If you just ack at the end of the poll function, you need
> to check the rings again to ensure you did not miss an interrupt
> between checking observing both rings to be empty and the irq-ack.
>
> I suspect it's still cheaper to check the two rings with an uncached
> read from memory than to to do the read-modify-write on the mmio,
> but you'd have to measure that to be sure.
>
Unfortunately the PHY on the board I have is 100Mbps which is the
limiting factor in benchmarking this driver. :(
If you're fine with this - I'd like to fix the minor issues you
pointed out and stick with the current approach for now. We can always
fix the implementation in the future once a board with a Gigabit PHY
is out. Most ethernet drivers don't use such fine-grained interrupt
control anyway. I expect the performance differences to be miniscule
really.
Bart
^ permalink raw reply
* Re: [PATCH v4 06/11] net: ethernet: mtk-eth-mac: new driver
From: Arnd Bergmann @ 2020-05-22 7:49 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Stephane Le Provost, Pedro Tsai, Andrew Perepech, Rob Herring,
David S . Miller, Matthias Brugger, John Crispin, Sean Wang,
Mark Lee, Jakub Kicinski, Fabien Parent, Heiner Kallweit,
Edwin Peer, DTML, linux-kernel@vger.kernel.org, Networking,
Linux ARM, moderated list:ARM/Mediatek SoC...,
Bartosz Golaszewski
In-Reply-To: <CAMRc=MfVkbDSfEV71SD57dpYthdx5epD0FOvjRx8qQGT+SgsTQ@mail.gmail.com>
On Fri, May 22, 2020 at 9:44 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> śr., 20 maj 2020 o 23:23 Arnd Bergmann <arnd@arndb.de> napisał(a):
> > On Wed, May 20, 2020 at 7:35 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > śr., 20 maj 2020 o 16:37 Arnd Bergmann <arnd@arndb.de> napisał(a):
> > > My thinking was this: if I mask the relevant interrupt (TX/RX
> > > complete) and ack it right away, the status bit will be asserted on
> > > the next packet received/sent but the process won't get interrupted
> > > and when I unmask it, it will fire right away and I won't have to
> > > recheck the status register. I noticed that if I ack it at the end of
> > > napi poll callback, I end up missing certain TX complete interrupts
> > > and end up seeing a lot of retransmissions even if I reread the status
> > > register. I'm not yet sure where this race happens.
> >
> > Right, I see. If you just ack at the end of the poll function, you need
> > to check the rings again to ensure you did not miss an interrupt
> > between checking observing both rings to be empty and the irq-ack.
> >
> > I suspect it's still cheaper to check the two rings with an uncached
> > read from memory than to to do the read-modify-write on the mmio,
> > but you'd have to measure that to be sure.
> >
>
> Unfortunately the PHY on the board I have is 100Mbps which is the
> limiting factor in benchmarking this driver. :(
>
> If you're fine with this - I'd like to fix the minor issues you
> pointed out and stick with the current approach for now. We can always
> fix the implementation in the future once a board with a Gigabit PHY
> is out. Most ethernet drivers don't use such fine-grained interrupt
> control anyway. I expect the performance differences to be miniscule
> really.
Ok, fair enough. The BQL limiting is the part that matters the most
for performance on slow lines (preventing long latencies from
buffer bloat), and you have that now.
Arnd
^ permalink raw reply
* Re: [PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA
From: Feng Tang @ 2020-05-22 7:58 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Grant Likely, Vinod Koul, Alan Cox,
Linus Walleij, Georgy Vlasov, Ramil Zaripov, Alexey Malahov,
Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
Andy Shevchenko, Rob Herring, linux-mips, devicetree,
Jarkko Nikula, Thomas Gleixner, Wan Ahmad Zainie, Linus Walleij,
Clement Leger, linux-spi, linux-kernel
In-Reply-To: <20200521153317.7wjp2r47q75fm6ge@mobilestation>
Hi Serge,
On Thu, May 21, 2020 at 06:33:17PM +0300, Serge Semin wrote:
> > > > > + dw_spi_dma_wait_rx_done(dws);
> > > >
> > > > I can understand the problem about TX, but I don't see how RX
> > > > will get hurt, can you elaborate more? thanks
> > > >
> > > > - Feng
> > >
> > > Your question is correct. You are right with your hypothesis. Ideally upon the
> > > dw_spi_dma_rx_done() execution Rx FIFO must be already empty. That's why the
> > > commit log signifies the error being mostly related with Tx FIFO. But
> > > practically there are many reasons why Rx FIFO might be left with data:
> > > DMA engine failures, incorrect DMA configuration (if DW SPI or DW DMA driver
> > > messed something up), controller hanging up, and so on. It's better to catch
> > > an error at this stage while propagating it up to the SPI device drivers.
> > > Especially seeing the wait-check implementation doesn't gives us much of the
> > > execution overhead in normal conditions. So by calling dw_spi_dma_wait_rx_done()
> > > we make sure that all the data has been fetched and we may freely get the
> > > buffers back to the client driver.
> >
> > I see your point about checking RX. But I still don't think checking
> > RX FIFO level is the right way to detect error. Some data left in
> > RX FIFO doesn't always mean a error, say for some case if there is
> > 20 words in RX FIFO, and the driver starts a DMA request for 16
> > words, then after a sucessful DMA transaction, there are 4 words
> > left without any error.
>
> Neither Tx nor Rx FIFO should be left with any data after transaction is
> finished. If they are then something has been wrong.
>
> See, every SPI transfer starts with FIFO clearance since we disable/enable the
> SPI controller by means of the SSIENR (spi_enable_chip(dws, 0) and
> spi_enable_chip(dws, 1) called in the dw_spi_transfer_one() callback). Here is the
> SSIENR register description: "It enables and disables all SPI Controller operations.
> When disabled, all serial transfers are halted immediately. Transmit and receive
> FIFO buffers are cleared when the device is disabled. It is impossible to program
> some of the SPI Controller control registers when enabled"
>
> No mater whether we start DMA request or perform the normal IRQ-based PIO, we
> request as much data as we need and neither Tx nor Rx FIFO are supposed to
> be left with any data after the request is finished. If data is left, then
> either we didn't push all of the necessary data to the SPI bus, or we didn't
> pull all the data from the FIFO, and this could have happened only due to some
> component mulfunction (drivers, DMA engine, SPI device). In any case the SPI
> device driver should be notified about the problem.
Data left in TX FIFO and Data left in RX FIFO are 2 different stories. The
former in dma case means the dma hw/driver has done its job, and spi hw/driver
hasn't done its job of pushing out the data to spi slave devices, while the
latter means the spi hw/driver has done its job, while the dma hw/driver hasn't.
And the code is called inside the dma rx channel callback, which means the
dma driver is saying "hey, I've done my job", but apparently it hasn't if
there is data left.
As for the wait time
+ nents = dw_readl(dws, DW_SPI_RXFLR);
+ ns = (NSEC_PER_SEC / spi_get_clk(dws)) * nents * dws->n_bytes *
+ BITS_PER_BYTE;
Using this formula for checking TX makes sense, but it doesn't for RX.
Because the time of pushing data in TX FIFO to spi device depends on
the clk, but the time of transferring RX FIFO to memory is up to
the DMA controller and peripheral bus.
Also for the
+ while (dw_spi_dma_rx_busy(dws) && retry--)
+ ndelay(ns);
+
the rx busy bit is cleared after this rx/tx checking, and it should
be always true at this point. Am I mis-reading the code?
Thanks,
Feng
>
> -Sergey
>
^ permalink raw reply
* Re: linux-next: build warning after merge of the aspeed tree
From: Arnd Bergmann @ 2020-05-22 8:15 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Olof Johansson, ARM, Joel Stanley, Rob Herring, devicetree,
Devicetree Compiler, Linux Next Mailing List,
Linux Kernel Mailing List, Manikandan Elumalai, Andrew Jeffery,
Vijay Khemka, David Gibson
In-Reply-To: <20200522101638.052bd0a2@canb.auug.org.au>
On Fri, May 22, 2020 at 2:16 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> On Wed, 20 May 2020 07:56:36 +0000 Joel Stanley <joel@jms.id.au> wrote:
> > On Mon, 11 May 2020 at 15:19, Rob Herring <robh+dt@kernel.org> wrote:
> > > On Wed, 6 May 2020 at 23:13, Joel Stanley <joel@jms.id.au> wrote:
> > > > > These are IPMB nodes with the SLAVE_ADDRESS bit set:
> > > > >
> > > > > +&i2c5 {
> > > > > + //Host3 IPMB bus
> > > > > + status = "okay";
> > > > > + multi-master;
> > > > > + ipmb5@10 {
> > > > > + compatible = "ipmb-dev";
> > > > > + reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
> > > > > + i2c-protocol;
> > > > > + };
> > > > >
> > > > > This is a correct entry, so dtc should not warn about it.
> > > >
> > > > I sent a patch for dtc here:
> > > > https://lore.kernel.org/lkml/20200508063904.60162-1-joel@jms.id.au/
> > >
> > > Patches for dtc need to be against upstream dtc. There's already a
> > > similar patch posted for it which I commented on and never saw a
> > > respin.
> >
> > Can I suggest some instructions in scsripts/dtc explaining that you
> > don't take patches in the kernel tree for this code?
> >
> > I've sent the patch so it applies to the dtc tree. It would be good to
> > see that change propagate over to -next as others have reported this
> > warning.
>
> These warnings now appear in the arm-soc tree.
Right, I also saw them earlier.
Joel, have you sent your patch to David Gibson for integration into
upstream dtc?
I don't know who sent the other patch, but as long as one of them
gets merged, I'd hope we can pull that into kernel as well.
Arnd
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml
From: Jonathan Albrieux @ 2020-05-22 8:22 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, linux-kernel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Rob Herring, Daniel Baluta
In-Reply-To: <20200521192736.4818f17b@archlinux>
On Thu, May 21, 2020 at 07:27:36PM +0100, Jonathan Cameron wrote:
> On Wed, 20 May 2020 09:24:23 +0200
> Jonathan Albrieux <jonathan.albrieux@gmail.com> wrote:
>
> > On Tue, May 19, 2020 at 06:49:33PM +0100, Jonathan Cameron wrote:
> > > On Tue, 19 May 2020 09:50:57 +0200
> > > Jonathan Albrieux <jonathan.albrieux@gmail.com> wrote:
> > >
> > > > Converts documentation from txt format to yaml
> > > >
> > > > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
> > > > ---
> > > > .../devicetree/bindings/iio/imu/bmi160.txt | 37 --------
> > > > .../devicetree/bindings/iio/imu/bmi160.yaml | 84 +++++++++++++++++++
> > > > 2 files changed, 84 insertions(+), 37 deletions(-)
> > > > delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
> > > > create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
> > > > deleted file mode 100644
> > > > index 900c169de00f..000000000000
> > > > --- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
> > > > +++ /dev/null
> > > > @@ -1,37 +0,0 @@
> > > > -Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
> > > > -and externally connectable Magnetometer
> > > > -
> > > > -https://www.bosch-sensortec.com/bst/products/all_products/bmi160
> > > > -
> > > > -Required properties:
> > > > - - compatible : should be "bosch,bmi160"
> > > > - - reg : the I2C address or SPI chip select number of the sensor
> > > > - - spi-max-frequency : set maximum clock frequency (only for SPI)
> > > > -
> > > > -Optional properties:
> > > > - - interrupts : interrupt mapping for IRQ
> > > > - - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
> > > > - input, set to "INT2" if INT2 pin should be used instead
> > > > - - drive-open-drain : set if the specified interrupt pin should be configured as
> > > > - open drain. If not set, defaults to push-pull.
> > > > -
> > > > -Examples:
> > > > -
> > > > -bmi160@68 {
> > > > - compatible = "bosch,bmi160";
> > > > - reg = <0x68>;
> > > > -
> > > > - interrupt-parent = <&gpio4>;
> > > > - interrupts = <12 IRQ_TYPE_EDGE_RISING>;
> > > > - interrupt-names = "INT1";
> > > > -};
> > > > -
> > > > -bmi160@0 {
> > > > - compatible = "bosch,bmi160";
> > > > - reg = <0>;
> > > > - spi-max-frequency = <10000000>;
> > > > -
> > > > - interrupt-parent = <&gpio2>;
> > > > - interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> > > > - interrupt-names = "INT2";
> > > > -};
> > > > diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bmi160.yaml
> > > > new file mode 100644
> > > > index 000000000000..6b464ce5ed0b
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/iio/imu/bmi160.yaml
> > > > @@ -0,0 +1,84 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Bosch BMI160
> > > > +
> > > > +maintainers:
> > > > + - can't find a mantainer, author is Daniel Baluta <daniel.baluta@intel.com>
> > >
> > > Daniel is still active in the kernel, just not at Intel any more. +CC
> > >
> >
> > Oh ok thank you! Daniel are you still maintaining this driver?
> >
> > > > +
> > > > +description: |
> > > > + Inertial Measurement Unit with Accelerometer, Gyroscope and externally
> > > > + connectable Magnetometer
> > > > + https://www.bosch-sensortec.com/bst/products/all_products/bmi160
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: bosch,bmi160
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > + description: the I2C address or SPI chip select number of the sensor
> > >
> > > As standard for i2c and spi, usually no need to have a description line for
> > > this element.
> > >
> >
> > Thank you, will remove the description then.
> >
> > > > +
> > > > + spi-max-frequency:
> > > > + maxItems: 1
> > > > + description: set maximum clock frequency (required only for SPI)
> > >
> > > Standard spi binding. Probably doesn't need to be included here.
> > >
> >
> > So should I completely remove it from properties?
>
> Yes
>
>
> Thanks,
>
> Jonathan
Ok I will remove it completely in next patch,
Thank you,
Best regards,
Jonathan Albrieux
^ permalink raw reply
* Re: [PATCH v2 3/4] iio: imu: bmi160: added regulator support
From: Jonathan Albrieux @ 2020-05-22 8:25 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, linux-kernel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler
In-Reply-To: <20200521193055.7ee7cf9c@archlinux>
On Thu, May 21, 2020 at 07:30:55PM +0100, Jonathan Cameron wrote:
> On Wed, 20 May 2020 09:17:51 +0200
> Jonathan Albrieux <jonathan.albrieux@gmail.com> wrote:
>
> > On Tue, May 19, 2020 at 06:55:35PM +0100, Jonathan Cameron wrote:
> > > On Tue, 19 May 2020 09:50:59 +0200
> > > Jonathan Albrieux <jonathan.albrieux@gmail.com> wrote:
> > >
> > > > v2: fixed missing description
> > >
> > > Don't put change log here....
> >
> > Yep I will put it in the cover letter
> >
> > > >
> > > > Add vdd-supply and vddio-supply support. Without this support vdd and vddio
> > > > should be set to always-on in device tree
> > >
> > > Kind of the opposite. If they are always on we don't have to provide them
> > > in the device tree.
> > >
> >
> > I wrote that because, testing on msm8916, without setting the regulators to
> > always on they were controlled by other components and it happened that
> > the line wasn't ready during probe causing failure to load the module.
> >
> > I will try to reword based on your comment, thank you.
>
> Ah. Understood. I'd give that explicit example in the patch description.
> I'd assumed this was the normal case of they weren't being described
> at all in DT, whereas you case is more complex.
>
> Jonathan
>
Yep, I omitted to describe the case I was in. I'll add it to next patch, thank
you,
> >
> > > A few trivial things inline.
> > >
> > > >
> > > > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
> > > > ---
> > >
> > > Change log goes here so we don't end up keeping it in the git log.
> > >
> > > > drivers/iio/imu/bmi160/bmi160.h | 2 ++
> > > > drivers/iio/imu/bmi160/bmi160_core.c | 27 ++++++++++++++++++++++++++-
> > > > 2 files changed, 28 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
> > > > index 621f5309d735..923c3b274fde 100644
> > > > --- a/drivers/iio/imu/bmi160/bmi160.h
> > > > +++ b/drivers/iio/imu/bmi160/bmi160.h
> > > > @@ -3,10 +3,12 @@
> > > > #define BMI160_H_
> > > >
> > > > #include <linux/iio/iio.h>
> > > > +#include <linux/regulator/consumer.h>
> > > >
> > > > struct bmi160_data {
> > > > struct regmap *regmap;
> > > > struct iio_trigger *trig;
> > > > + struct regulator_bulk_data supplies[2];
> > > > };
> > > >
> > > > extern const struct regmap_config bmi160_regmap_config;
> > > > diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> > > > index 6af65d6f1d28..9bbe0d8e6720 100644
> > > > --- a/drivers/iio/imu/bmi160/bmi160_core.c
> > > > +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> > > > @@ -15,6 +15,7 @@
> > > > #include <linux/delay.h>
> > > > #include <linux/irq.h>
> > > > #include <linux/of_irq.h>
> > > > +#include <linux/regulator/consumer.h>
> > > >
> > > > #include <linux/iio/iio.h>
> > > > #include <linux/iio/triggered_buffer.h>
> > > > @@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
> > > > unsigned int val;
> > > > struct device *dev = regmap_get_device(data->regmap);
> > > >
> > > > + ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
> > > > + if (ret) {
> > > > + dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > > > + return ret;
> > > > + }
> > > > +
> > > > ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
> > > > if (ret)
> > > > return ret;
> > > > @@ -793,9 +800,17 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
> > > > static void bmi160_chip_uninit(void *data)
> > > > {
> > > > struct bmi160_data *bmi_data = data;
> > > > + struct device *dev = regmap_get_device(bmi_data->regmap);
> > > > + int ret;
> > > >
> > > > bmi160_set_mode(bmi_data, BMI160_GYRO, false);
> > > > bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
> > > > +
> > > > + ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
> > > > + bmi_data->supplies);
> > > > + if (ret) {
> > > > + dev_err(dev, "Failed to disable regulators: %d\n", ret);
> > > > + }
> > > No need for brackets around a 1 line if block
> > >
> >
> > Thank you, I didn't noticed that :-)
> >
> > > if (ret)
> > > dev_err(dev, "failed to disable regulators: %d\n", ret);
> > >
> > > > }
> > > >
> > > > int bmi160_core_probe(struct device *dev, struct regmap *regmap,
> > > > @@ -815,6 +830,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
> > > > dev_set_drvdata(dev, indio_dev);
> > > > data->regmap = regmap;
> > > >
> > > > + data->supplies[0].supply = "vdd";
> > > > + data->supplies[1].supply = "vddio";
> > > > + ret = devm_regulator_bulk_get(dev,
> > > > + ARRAY_SIZE(data->supplies),
> > > > + data->supplies);
> > > > + if (ret) {
> > > > + dev_err(dev, "Failed to get regulators: %d\n", ret);
> > > > + return ret;
> > > > + }
> > > > +
> > > > ret = bmi160_chip_init(data, use_spi);
> > > > if (ret)
> > > > return ret;
> > > > @@ -853,6 +878,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
> > > > }
> > > > EXPORT_SYMBOL_GPL(bmi160_core_probe);
> > > >
> > > > -MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com");
> > > > +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
> > >
> > > Good fix but shouldn't be in this patch. Put it a separate patch on it's own.
> > >
> >
> > Ok will separate this fix into another patch, thank you!
> >
> > > > MODULE_DESCRIPTION("Bosch BMI160 driver");
> > > > MODULE_LICENSE("GPL v2");
> > >
> > >
> >
> > Best regards,
> > Jonathan Albrieux
>
Best regards,
Jonathan Albrieux
^ permalink raw reply
* Re: [PATCH v1 2/2] Add PWM driver for LGM
From: Uwe Kleine-König @ 2020-05-22 8:56 UTC (permalink / raw)
To: Rahul Tanwar
Cc: thierry.reding, p.zabel, linux-pwm, robh+dt, linux-kernel,
devicetree, andriy.shevchenko, songjun.Wu, cheol.yong.kim,
qi-ming.wu
In-Reply-To: <3c1d2343b034325dbc185ccd23a35b40a62a4e7b.1590132733.git.rahul.tanwar@linux.intel.com>
Hello,
On Fri, May 22, 2020 at 03:41:59PM +0800, Rahul Tanwar wrote:
> Add PWM controller driver for Intel's Lightning Mountain(LGM) SoC.
>
> Signed-off-by: Rahul Tanwar <rahul.tanwar@linux.intel.com>
> ---
> drivers/pwm/Kconfig | 9 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-intel-lgm.c | 356 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 366 insertions(+)
> create mode 100644 drivers/pwm/pwm-intel-lgm.c
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index eebbc917ac97..a582214f50b2 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -232,6 +232,15 @@ config PWM_IMX_TPM
> To compile this driver as a module, choose M here: the module
> will be called pwm-imx-tpm.
>
> +config PWM_INTEL_LGM
> + tristate "Intel LGM PWM support"
> + depends on X86 || COMPILE_TEST
> + help
> + Generic PWM framework driver for LGM SoC.
I wouldn't call this "framework".
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-intel-lgm.
> +
> config PWM_JZ4740
> tristate "Ingenic JZ47xx PWM support"
> depends on MACH_INGENIC
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 9a475073dafc..c16a972a101d 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_PWM_IMG) += pwm-img.o
> obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o
> obj-$(CONFIG_PWM_IMX27) += pwm-imx27.o
> obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o
> +obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o
> obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o
> obj-$(CONFIG_PWM_LP3943) += pwm-lp3943.o
> obj-$(CONFIG_PWM_LPC18XX_SCT) += pwm-lpc18xx-sct.o
> diff --git a/drivers/pwm/pwm-intel-lgm.c b/drivers/pwm/pwm-intel-lgm.c
> new file mode 100644
> index 000000000000..e307fd2457df
> --- /dev/null
> +++ b/drivers/pwm/pwm-intel-lgm.c
> @@ -0,0 +1,356 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Intel Corporation.
> + */
Please point out limitations of your driver here similar to e.g.
drivers/pwm/pwm-sifive.c (and in the same format).
You should mention the fixed period of the hardware here (assuming I
interpreted this correctly below) and missing support for polarity.
How does the PWM behave on disable? Does the output become inactive
then? Does it complete the currently running period? What about
reconfiguration? Does changing the duty cycle complete the currently
running period?
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#define PWM_FAN_CON0 0x0
> +#define PWM_FAN_EN_EN BIT(0)
> +#define PWM_FAN_EN_DIS 0x0
> +#define PWM_FAN_EN_MSK BIT(0)
> +#define PWM_FAN_MODE_2WIRE 0x0
> +#define PWM_FAN_MODE_4WIRE 0x1
> +#define PWM_FAN_MODE_MSK BIT(1)
> +#define PWM_FAN_PWM_DIS_DIS 0x0
> +#define PWM_FAN_PWM_DIS_MSK BIT(2)
> +#define PWM_TACH_EN_EN 0x1
> +#define PWM_TACH_EN_MSK BIT(4)
> +#define PWM_TACH_PLUS_2 0x0
> +#define PWM_TACH_PLUS_4 0x1
> +#define PWM_TACH_PLUS_MSK BIT(5)
> +#define PWM_FAN_DC_MSK GENMASK(23, 16)
> +
> +#define PWM_FAN_CON1 0x4
> +#define PWM_FAN_MAX_RPM_MSK GENMASK(15, 0)
> +
> +#define PWM_FAN_STAT 0x10
> +#define PWM_FAN_TACH_MASK GENMASK(15, 0)
> +
> +#define MAX_RPM (BIT(16) - 1)
> +#define DFAULT_RPM 4000
> +#define MAX_DUTY_CYCLE (BIT(8) - 1)
> +
> +#define FRAC_BITS 10
> +#define TWO_TENTH 204
> +
> +#define TWO_SECONDS 2000
> +#define IGNORE_FIRST_ERR 1
> +#define THIRTY_SECS_WINDOW 15
> +#define ERR_CNT_THRESHOLD 6
> +
> +struct intel_pwm_chip {
> + struct pwm_chip chip;
> + struct regmap *regmap;
> + struct clk *clk;
> + struct reset_control *rst;
> + u32 tach_en;
> + u32 max_rpm;
> + u32 set_rpm;
> + u32 set_dc;
> + struct delayed_work work;
> +};
I don't like aligning the member names and prefer a single space here.
(But I'm aware this is subjective and others like it the why you did
it.)
> +
> +static inline struct intel_pwm_chip *to_intel_pwm_chip(struct pwm_chip *chip)
> +{
> + return container_of(chip, struct intel_pwm_chip, chip);
> +}
> +
> +static int pwm_update_dc(struct intel_pwm_chip *pc, u32 val)
Please use a common function prefix for all functions in your driver.
> +{
> + return regmap_update_bits(pc->regmap, PWM_FAN_CON0, PWM_FAN_DC_MSK,
> + FIELD_PREP(PWM_FAN_DC_MSK, val));
> +}
> +
> +static int intel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> + int duty_ns, int period_ns)
> +{
> + struct intel_pwm_chip *pc = to_intel_pwm_chip(chip);
> + u32 val;
> +
> + val = DIV_ROUND_CLOSEST(duty_ns * MAX_DUTY_CYCLE, period_ns);
> + val = min_t(u32, val, MAX_DUTY_CYCLE);
The period is fixed for this hardware type? If so you're supposed to
refuse requests for a period smaller than the actual period length.
Also duty_cycle is supposed to round down.
(That's something that PWM_DEBUG will tell you, too)
> + if (pc->tach_en) {
> + pc->set_dc = val;
> + pc->set_rpm = val * pc->max_rpm / MAX_DUTY_CYCLE;
> + }
> +
> + return pwm_update_dc(pc, val);
> +}
> +
> +static int intel_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct intel_pwm_chip *pc = to_intel_pwm_chip(chip);
> + struct regmap *regmap = pc->regmap;
> +
> + regmap_update_bits(regmap, PWM_FAN_CON0,
> + PWM_FAN_EN_MSK, PWM_FAN_EN_EN);
> +
> + if (pc->tach_en)
> + schedule_delayed_work(&pc->work, msecs_to_jiffies(10000));
> +
> + return 0;
> +}
> +
> +static void intel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct intel_pwm_chip *pc = to_intel_pwm_chip(chip);
> + struct regmap *regmap = pc->regmap;
> +
> + if (pc->tach_en)
> + cancel_delayed_work_sync(&pc->work);
> +
> + regmap_update_bits(regmap, PWM_FAN_CON0,
> + PWM_FAN_EN_MSK, PWM_FAN_EN_DIS);
> +}
> +
> +static const struct pwm_ops intel_pwm_ops = {
> + .config = intel_pwm_config,
> + .enable = intel_pwm_enable,
> + .disable = intel_pwm_disable,
> + .owner = THIS_MODULE,
Please implement .apply instead of .config/.enable/.disable. Please also
enable PWM_DEBUG and fix the issues pointed out with it.
> +};
> +
> +static void tach_work(struct work_struct *work)
> +{
> + struct intel_pwm_chip *pc = container_of(work, struct intel_pwm_chip,
> + work.work);
> + struct regmap *regmap = pc->regmap;
> + u32 fan_tach, fan_dc, val;
> + s32 diff;
> + static u32 fanspeed_err_cnt, time_window, delta_dc;
> +
> + /*
> + * Fan speed is tracked by reading the active duty cycle of PWM output
> + * from the active duty cycle register. Some variance in the duty cycle
> + * register value is expected. So we set a time window of 30 seconds and
> + * if we detect inaccurate fan speed 6 times within 30 seconds then we
> + * mark it as fan speed problem and fix it by readjusting the duty cycle.
> + */
I'm a unhappy to have this in the PWM driver. The PWM driver is supposed
to be generic and I think this belongs into a dedicated driver.
> +
> + if (fanspeed_err_cnt > IGNORE_FIRST_ERR)
> + /*
> + * Ignore first time we detect inaccurate fan speed
> + * because it is expected during bootup.
> + */
> + time_window++;
> +
> + if (time_window == THIRTY_SECS_WINDOW) {
> + /*
> + * This work is scheduled every 2 seconds i.e. each time_window
> + * counter step roughly mean 2 seconds. When the time window
> + * reaches 30 seconds, reset all the counters/logic.
> + */
> + fanspeed_err_cnt = 0;
> + delta_dc = 0;
> + time_window = 0;
> + }
> +
> + regmap_read(regmap, PWM_FAN_STAT, &fan_tach);
> + fan_tach &= PWM_FAN_TACH_MASK;
> + if (!fan_tach)
> + goto restart_work;
> +
> + val = DIV_ROUND_CLOSEST(pc->set_rpm << FRAC_BITS, fan_tach);
> + diff = val - BIT(FRAC_BITS);
> +
> + if (abs(diff) > TWO_TENTH) {
> + /* if duty cycle diff is more than two tenth, detect it as error */
> + if (fanspeed_err_cnt > IGNORE_FIRST_ERR)
> + delta_dc += val;
> + fanspeed_err_cnt++;
> + }
> +
> + if (fanspeed_err_cnt == ERR_CNT_THRESHOLD) {
> + /*
> + * We detected fan speed errors 6 times with 30 seconds.
> + * Fix the error by readjusting duty cycle and reset
> + * our counters/logic.
> + */
> + fan_dc = pc->set_dc * delta_dc >> (FRAC_BITS + 2);
> + fan_dc = min_t(u32, fan_dc, MAX_DUTY_CYCLE);
> + pwm_update_dc(pc, fan_dc);
> + fanspeed_err_cnt = 0;
> + delta_dc = 0;
> + time_window = 0;
> + }
> +
> +restart_work:
> + /*
> + * Fan speed doesn't need continous tracking. Schedule this work
> + * every two seconds so it doesn't steal too much cpu cycles.
> + */
> + schedule_delayed_work(&pc->work, msecs_to_jiffies(TWO_SECONDS));
> +}
> +
> +static void pwm_init(struct intel_pwm_chip *pc)
> +{
> + struct device *dev = pc->chip.dev;
> + struct regmap *regmap = pc->regmap;
> + u32 max_rpm, fan_wire, tach_plus, con0_val, con0_mask;
> +
> + if (device_property_read_u32(dev, "intel,fan-wire", &fan_wire))
> + fan_wire = 2; /* default is 2 wire mode */
> +
> + con0_val = FIELD_PREP(PWM_FAN_PWM_DIS_MSK, PWM_FAN_PWM_DIS_DIS);
> + con0_mask = PWM_FAN_PWM_DIS_MSK | PWM_FAN_MODE_MSK;
> +
> + switch (fan_wire) {
> + case 2 ... 3:
> + con0_val |= FIELD_PREP(PWM_FAN_MODE_MSK, PWM_FAN_MODE_2WIRE);
> + break;
This can be dropped as it matches the default case.
> + case 4:
> + con0_val |= FIELD_PREP(PWM_FAN_MODE_MSK, PWM_FAN_MODE_4WIRE) |
> + FIELD_PREP(PWM_TACH_EN_MSK, PWM_TACH_EN_EN);
> + con0_mask |= PWM_TACH_EN_MSK | PWM_TACH_PLUS_MSK;
> + pc->tach_en = 1;
> + break;
> + default:
> + /* default is 2wire mode */
> + con0_val |= FIELD_PREP(PWM_FAN_MODE_MSK, PWM_FAN_MODE_2WIRE);
> + break;
> + }
> +
> + if (pc->tach_en) {
> + if (device_property_read_u32(dev, "intel,tach-plus",
> + &tach_plus))
> + tach_plus = 2;
> +
> + switch (tach_plus) {
> + case 2:
> + con0_val |= FIELD_PREP(PWM_TACH_PLUS_MSK,
> + PWM_TACH_PLUS_2);
> + break;
> + case 4:
> + con0_val |= FIELD_PREP(PWM_TACH_PLUS_MSK,
> + PWM_TACH_PLUS_4);
> + break;
> + default:
> + con0_val |= FIELD_PREP(PWM_TACH_PLUS_MSK,
> + PWM_TACH_PLUS_2);
> + break;
Similarily here, case 2: and default are identical.
> + }
> +
> + if (device_property_read_u32(dev, "intel,max-rpm", &max_rpm))
> + max_rpm = DFAULT_RPM;
> +
> + max_rpm = min_t(u32, max_rpm, MAX_RPM);
> + if (max_rpm == 0)
> + max_rpm = DFAULT_RPM;
> +
> + pc->max_rpm = max_rpm;
> + INIT_DEFERRABLE_WORK(&pc->work, tach_work);
> + regmap_update_bits(regmap, PWM_FAN_CON1,
> + PWM_FAN_MAX_RPM_MSK, max_rpm);
> + }
> +
> + regmap_update_bits(regmap, PWM_FAN_CON0, con0_mask, con0_val);
> +}
> +
> +static const struct regmap_config pwm_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> +};
> +
> +static int intel_pwm_probe(struct platform_device *pdev)
> +{
> + struct intel_pwm_chip *pc;
> + struct device *dev = &pdev->dev;
> + void __iomem *io_base;
> + int ret;
> +
> + pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
> + if (!pc)
> + return -ENOMEM;
> +
> + io_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(io_base))
error message here?
> + return PTR_ERR(io_base);
> +
> + pc->regmap = devm_regmap_init_mmio(dev, io_base, &pwm_regmap_config);
> + if (IS_ERR(pc->regmap)) {
> + ret = PTR_ERR(pc->regmap);
> + dev_err(dev, "failed to init register map: %d\n", ret);
Better use %pe for the error code to get a more descriptive message.
> + return ret;
> + }
> +
> + pc->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(pc->clk)) {
> + ret = PTR_ERR(pc->clk);
> + dev_err(dev, "failed to get clock: %d\n", ret);
> + return ret;
> + }
> +
> + pc->rst = devm_reset_control_get(dev, NULL);
> + if (IS_ERR(pc->rst)) {
> + ret = PTR_ERR(pc->rst);
> + dev_err(dev, "failed to get reset control: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(pc->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable clock\n");
> + return ret;
> + }
> +
> + reset_control_deassert(pc->rst);
return value check is missing.
> + pc->chip.dev = dev;
> + pc->chip.ops = &intel_pwm_ops;
> + pc->chip.npwm = 1;
> +
> + pwm_init(pc);
> +
> + ret = pwmchip_add(&pc->chip);
> + if (ret < 0) {
> + dev_err(dev, "failed to add PWM chip: %d\n", ret);
> + clk_disable_unprepare(pc->clk);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, pc);
> + return 0;
> +}
> +
> +static int intel_pwm_remove(struct platform_device *pdev)
> +{
> + struct intel_pwm_chip *pc = platform_get_drvdata(pdev);
> + int ret;
> +
> + if (pc->tach_en)
> + cancel_delayed_work_sync(&pc->work);
> +
> + ret = pwmchip_remove(&pc->chip);
> + if (ret < 0)
> + return ret;
> +
> + reset_control_assert(pc->rst);
> +
> + clk_disable_unprepare(pc->clk);
In .probe() you first release reset and then enable the clock. It's good
style to do it the other way round in .remove().
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ 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