* Re: [PATCH] irqchip: ativic32: constify irq_domain_ops
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, Masahiro Yamada
Cc: linux-kernel, marex, linux-arm-kernel, linux-gpio, linux-stm32
In-Reply-To: <20200714173857.477422-1-masahiroy@kernel.org>
On Wed, 15 Jul 2020 02:38:57 +0900, Masahiro Yamada wrote:
> This is passed to irq_domain_add_linear(), which accepts a pointer
> to a const structure.
Applied to irq/irqchip-5.9, thanks!
[1/1] irqchip/ativic32: Constify irq_domain_ops
commit: 605a2cf566e130c77fc2cc77fac37fb901fc868a
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v3 0/3] Allow for qcom-pdc to be loadable as a module
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, John Stultz,
lkml
Cc: marex, linux-arm-kernel, linux-gpio, linux-stm32, Joerg Roedel,
Andy Gross, Todd Kjos, iommu, Linus Walleij, Greg Kroah-Hartman,
Maulik Shah, Lina Iyer, linux-arm-msm, Bjorn Andersson,
Saravana Kannan
In-Reply-To: <20200710231824.60699-1-john.stultz@linaro.org>
On Fri, 10 Jul 2020 23:18:21 +0000, John Stultz wrote:
> This patch series provides exports and config tweaks to allow
> the qcom-pdc driver to be able to be configured as a permement
> modules (particularlly useful for the Android Generic Kernel
> Image efforts).
>
> This was part of a larger patch series, to enable qcom_scm
> driver to be a module as well, but I've split it out as there
> are some outstanding objections I still need to address with
> the follow-on patches, and wanted to see if progress could be
> made on this subset of the series in the meantime.
>
> [...]
Applied to irq/irqchip-5.9, thanks!
[1/3] irqdomain: Export irq_domain_update_bus_token
commit: ce8cefa53c06cd98607125c52c91e74373a893a0
[2/3] genirq: Export irq_chip_retrigger_hierarchy and irq_chip_set_vcpu_affinity_parent
commit: 96703f046c42f8ab15e735953cbfee572c717e2d
[3/3] irqchip/qcom-pdc: Allow QCOM_PDC to be loadable as a permanent module
commit: 5ef7f1bbf9f56c5380c4d876655920cac92008e5
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH -next] irqchip: mips-gic: Make some symbols static
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, Wei Yongjun,
Hulk Robot
Cc: linux-kernel, marex, linux-arm-kernel, linux-gpio, linux-stm32
In-Reply-To: <20200714142245.16124-1-weiyongjun1@huawei.com>
On Tue, 14 Jul 2020 22:22:45 +0800, Wei Yongjun wrote:
> The sparse tool complains as follows:
>
> drivers/irqchip/irq-mips-gic.c:49:1: warning:
> symbol '__pcpu_scope_pcpu_masks' was not declared. Should it be static?
> drivers/irqchip/irq-mips-gic.c:620:6: warning:
> symbol 'gic_ipi_domain_free' was not declared. Should it be static?
> drivers/irqchip/irq-mips-gic.c:634:5: warning:
> symbol 'gic_ipi_domain_match' was not declared. Should it be static?
>
> [...]
Applied to irq/irqchip-5.9, thanks!
[1/1] irqchip/mips-gic: Make local symbols static
commit: 63bf3444359c94d647c2afa79b5e732585469581
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH] genirq/irqdomain: Remove redundant NULL pointer check on fwnode
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner, Zenghui Yu
Cc: linux-kernel, marex, linux-arm-kernel, linux-gpio, linux-stm32,
wanghaibin.wang
In-Reply-To: <20200716083905.287-1-yuzenghui@huawei.com>
On Thu, 16 Jul 2020 16:39:05 +0800, Zenghui Yu wrote:
> The is_fwnode_irqchip() helper will check if the fwnode_handle is empty.
> There is no need to perform a redundant check outside of it.
Applied to irq/irqchip-5.9, thanks!
[1/1] genirq/irqdomain: Remove redundant NULL pointer check on fwnode
commit: 47903428b0e9db7a6251aa696fd1b2fc5de98545
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [RESEND PATCH] irqchip/stm32-exti: Use the hwspin_lock_timeout_in_atomic() API
From: Marc Zyngier @ 2020-07-17 12:48 UTC (permalink / raw)
To: Alexandre Torgue, Jason Cooper, Thomas Gleixner
Cc: linux-kernel, marex, linux-arm-kernel, linux-gpio, linux-stm32
In-Reply-To: <20200706081115.25180-1-alexandre.torgue@st.com>
On Mon, 6 Jul 2020 10:11:15 +0200, Alexandre Torgue wrote:
> Now that the hwspin_lock_timeout_in_atomic() API is available use it.
>
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>
> diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
> index faa8482c8246..c7ab69694931 100644
> --- a/drivers/irqchip/irq-stm32-exti.c
> +++ b/drivers/irqchip/irq-stm32-exti.c
> @@ -25,7 +25,6 @@
> #define IRQS_PER_BANK 32
>
> [...]
Applied to irq/irqchip-5.9, thanks!
[1/1] irqchip/stm32-exti: Use the hwspin_lock_timeout_in_atomic() API
commit: e5c19cf32b68d8c59cd3e94e257ab030f07db7d6
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v2] gpio: omap: handle pin config bias flags
From: Drew Fustini @ 2020-07-17 12:45 UTC (permalink / raw)
To: Linus Walleij
Cc: Tony Lindgren, Haojian Zhuang, Grygorii Strashko, Linux-OMAP,
open list:GPIO SUBSYSTEM, Jason Kridner, Robert Nelson
In-Reply-To: <20200717124048.GA1765226@x1>
On Fri, Jul 17, 2020 at 02:40:48PM +0200, Drew Fustini wrote:
> On Thu, Jul 16, 2020 at 10:29:30AM +0200, Linus Walleij wrote:
> > On Wed, Jul 15, 2020 at 11:37 PM Drew Fustini <drew@beagleboard.org> wrote:
> >
> > > Modify omap_gpio_set_config() to handle pin config bias flags by calling
> > > gpiochip_generic_config().
> > >
> > > The pin group for the gpio line must have the corresponding pinconf
> > > properties:
> > >
> > > PIN_CONFIG_BIAS_PULL_UP requires "pinctrl-single,bias-pullup"
> > > PIN_CONFIG_BIAS_PULL_DOWN requires "pinctrl-single,bias-pulldown"
> > >
> > > This is necessary for pcs_pinconf_set() to find the requested bias
> > > parameter in the PIN_MAP_TYPE_CONFIGS_GROUP pinctrl map.
> > >
> > > Acked-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > > Acked-by: Tony Lindgren <tony@atomide.com>
> > > Signed-off-by: Drew Fustini <drew@beagleboard.org>
> >
> > This v2 version applied!
> >
> > Yours,
> > Linus Walleij
>
> Thanks!
>
> I'm curious which branch should I look in to find it?
>
> I didn't see it in any of the branches in:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/refs/
>
> -Drew
Oh, nevermind. I found it in linux-pinctrl:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/commit/?h=for-next&id=40e30d26d909af89de2dcd0b4abdd27c47ac2235
Thanks,
Drew
^ permalink raw reply
* Re: [PATCH v2] gpio: omap: handle pin config bias flags
From: Drew Fustini @ 2020-07-17 12:40 UTC (permalink / raw)
To: Linus Walleij
Cc: Tony Lindgren, Haojian Zhuang, Grygorii Strashko, Linux-OMAP,
open list:GPIO SUBSYSTEM, Jason Kridner, Robert Nelson
In-Reply-To: <CACRpkdZ+Bm4MsyaJJ89q7_KfgmyQWyJ57SC3F38gxTbsOfwNTA@mail.gmail.com>
On Thu, Jul 16, 2020 at 10:29:30AM +0200, Linus Walleij wrote:
> On Wed, Jul 15, 2020 at 11:37 PM Drew Fustini <drew@beagleboard.org> wrote:
>
> > Modify omap_gpio_set_config() to handle pin config bias flags by calling
> > gpiochip_generic_config().
> >
> > The pin group for the gpio line must have the corresponding pinconf
> > properties:
> >
> > PIN_CONFIG_BIAS_PULL_UP requires "pinctrl-single,bias-pullup"
> > PIN_CONFIG_BIAS_PULL_DOWN requires "pinctrl-single,bias-pulldown"
> >
> > This is necessary for pcs_pinconf_set() to find the requested bias
> > parameter in the PIN_MAP_TYPE_CONFIGS_GROUP pinctrl map.
> >
> > Acked-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > Signed-off-by: Drew Fustini <drew@beagleboard.org>
>
> This v2 version applied!
>
> Yours,
> Linus Walleij
Thanks!
I'm curious which branch should I look in to find it?
I didn't see it in any of the branches in:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/refs/
-Drew
^ permalink raw reply
* Re: [PATCH] irqchip/stm32-exti: map direct event to irq parent
From: Marc Zyngier @ 2020-07-17 12:37 UTC (permalink / raw)
To: Alexandre Torgue
Cc: kernel test robot, Thomas Gleixner, Jason Cooper, kbuild-all,
linux-arm-kernel, linux-kernel, linux-gpio, linux-stm32, marex
In-Reply-To: <69fb49b4-6a41-edf4-fea3-3f10934817ca@st.com>
On Fri, 10 Jul 2020 10:37:47 +0100,
Alexandre Torgue <alexandre.torgue@st.com> wrote:
>
> Hi Marc,
>
> On 7/10/20 11:31 AM, Marc Zyngier wrote:
> > Alexandre,
> >
> > On Wed, 08 Jul 2020 05:57:24 +0100,
> > kernel test robot <lkp@intel.com> wrote:
> >>
> >> [1 <text/plain; us-ascii (7bit)>]
> >> Hi Alexandre,
> >>
> >> I love your patch! Perhaps something to improve:
> >>
> >> [auto build test WARNING on stm32/stm32-next]
> >> [also build test WARNING on soc/for-next v5.8-rc4 next-20200707]
> >> [If your patch is applied to the wrong git tree, kindly drop us a note.
> >> And when submitting patch, we suggest to use as documented in
> >> https://git-scm.com/docs/git-format-patch]
> >>
> >> url: https://github.com/0day-ci/linux/commits/Alexandre-Torgue/irqchip-stm32-exti-map-direct-event-to-irq-parent/20200706-161327
> >> base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
> >> config: arm-randconfig-s031-20200707 (attached as .config)
> >> compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
> >> reproduce:
> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >> chmod +x ~/bin/make.cross
> >> # apt-get install sparse
> >> # sparse version: v0.6.2-31-gabbfd661-dirty
> >> # save the attached .config to linux build tree
> >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm
> >>
> >> If you fix the issue, kindly add following tag as appropriate
> >> Reported-by: kernel test robot <lkp@intel.com>
> >>
> >> All warnings (new ones prefixed by >>):
> >>
> >> In file included from include/linux/build_bug.h:5,
> >> from include/linux/bits.h:23,
> >> from include/linux/bitops.h:5,
> >> from drivers/irqchip/irq-stm32-exti.c:8:
> >> drivers/irqchip/irq-stm32-exti.c: In function 'stm32_exti_h_domain_alloc':
> >> drivers/irqchip/irq-stm32-exti.c:683:23: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> >> 683 | if (desc->irq_parent >= 0) {
> >> | ^~
> >> include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
> >> 58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
> >> | ^~~~
> >>>> drivers/irqchip/irq-stm32-exti.c:683:2: note: in expansion of macro 'if'
> >> 683 | if (desc->irq_parent >= 0) {
> >
> > Do you plan to address this? Looks like an actual bug to me.
>
> I'll fix it in v2, I was just waiting for other comments before
> sending a v2. Do you prefer I send a v2 with this fix, and you'll do
> your review on this v2 ?
I'm certainly not going to review something that has such a glaring
issue.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Geert Uytterhoeven @ 2020-07-17 12:27 UTC (permalink / raw)
To: Greg KH
Cc: Linus Walleij, Anson Huang, John Stultz, Russell King, Shawn Guo,
Sascha Hauer, Sascha Hauer, Fabio Estevam, Catalin Marinas,
Will Deacon, Bartosz Golaszewski, oleksandr.suvorov@toradex.com,
Adam Ford, Andreas Kemnade, hverkuil-cisco@xs4all.nl,
Bjorn Andersson, Leo Li, Vinod Koul, Linux ARM,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx, Jon Corbet, Arnd Bergmann, Olof Johansson,
Kevin Hilman
In-Reply-To: <20200717121436.GA2953399@kroah.com>
Hi Greg,
On Fri, Jul 17, 2020 at 2:14 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> Android is just finally pushing vendors to get their code upstream,
> which is a good thing to see. And building things as a module is an
> even better thing as now it is finally allowing arm64 systems to be
> built to support more than one specific hardware platform at runtime.
Can you please stop spreading this FUD?
As I said before, Arm64 kernels have supported more than one specific
hardware platform at runtime from the beginning of the arm64 port
(assumed the needed platform support has been enabled in the kernel
config, of course).
Even most arm32 kernels support this, since the introduction of the
CONFIG_ARCH_MULTIPLATFORM option. In fact every recently
introduced arm32 platform is usually v7, and must conform to this.
The sole exceptions are very old platforms, and the v4/v5/v6/v7 split
due to architectural issues (the latter still support clusters of
platforms in a single kernel).
Thank you, and have a nice weekend!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Greg KH @ 2020-07-17 12:14 UTC (permalink / raw)
To: Linus Walleij
Cc: Anson Huang, John Stultz, Russell King, Shawn Guo, Sascha Hauer,
Sascha Hauer, Fabio Estevam, Catalin Marinas, Will Deacon,
Bartosz Golaszewski, oleksandr.suvorov@toradex.com, Adam Ford,
Andreas Kemnade, hverkuil-cisco@xs4all.nl, Bjorn Andersson,
Leo Li, Vinod Koul, Geert Uytterhoeven, Olof Johansson, Linux ARM,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx, Jon Corbet
In-Reply-To: <CACRpkdbCVZaHqijqMck+jLAJuCAT31HA=9wwcV_EnQc=hsXLwg@mail.gmail.com>
On Fri, Jul 17, 2020 at 02:01:16PM +0200, Linus Walleij wrote:
> Greg, John,
>
> we need some guidance here. See below.
>
> On Thu, Jul 16, 2020 at 4:38 PM Anson Huang <anson.huang@nxp.com> wrote:
> > [Me]
> > > On Wed, Jul 15, 2020 at 4:44 AM Anson Huang <anson.huang@nxp.com>
>
> > > > I tried to replace the subsys_initcall() with
> > > > module_platform_driver(), but met issue about "
> > > > register_syscore_ops(&mxc_gpio_syscore_ops);" which is called in
> > > > gpio_mxc_init() function, this function should be called ONLY once,
> > > > moving it to .probe function is NOT working, so we may need to keep the
> > > > gpio_mxc_init(), that is another reason that we may need to keep
> > > > subsys_initcall()?
> > >
> > > This looks a bit dangerous to keep like this while allowing this code to be used
> > > from a module.
> > >
> > > What happens if you insmod and rmmod this a few times, really?
> > > How is this tested?
> > >
> > > This is not really modularized if that isn't working, just that modprobing once
> > > works isn't real modularization IMO, it seems more like a quick and dirty way
> > > to get Androids GKI somewhat working with the module while not properly
> > > making the module a module.
> > >
> > > You need input from the driver maintainers on how to handle this.
> >
> > As far as I know, some general/critical modules are NOT supporting rmmod, like
> > clk, pinctrl, gpio etc., and I am NOT sure whether Android GKI need to support
> > rmmod for these system-wide-used module, I will ask them for more detail about
> > this.
> >
> > The requirement I received is to support loadable module, but so far no hard requirement
> > to support module remove for gpio driver, so, is it OK to add it step by step, and this patch
> > series ONLY to support module build and one time modprobe?
>
> While I am a big fan of the Android GKI initiative this needs to be aligned
> with the Linux core maintainers, so let's ask Greg. I am also paging
> John Stultz on this: he is close to this action.
>
> They both know the Android people very well.
>
> So there is a rationale like this going on: in order to achieve GKI goals
> and have as much as possible of the Linux kernel stashed into loadable
> kernel modules, it has been elevated to modus operandi amongst
> the developers pushing this change that it is OK to pile up a load of
> modules that cannot ever be unloaded.
Why can't the module be unloaded? Is it just because they never
implement the proper "remove all resources allocated" logic in a remove
function, or something else?
> This is IIUC regardless of whether all consumers of the module are
> actually gone: it would be OK to say make it impossible to rmmod
> a clk driver even of zero clocks from that driver is in use. So it is not
> dependency-graph problem, it is a "load once, never remove" approach.
Sounds like a "lazy" approach :)
> This rationale puts me as subsystem maintainer in an unpleasant spot:
> it is really hard to tell case-to-case whether that change really is a
> technical advantage for the kernel per se or whether it is done for the
> greater ecosystem of Android.
>
> Often I would say it makes it possible to build a smaller kernel vmlinux
> so OK that is an advantage. On the other hand I have an inkling that I
> should be pushing developers to make sure that rmmod works.
I can see where a number of modules just can not ever be removed because
of resources and not being able to properly tear down, but that doesn't
mean that a driver author shouldn't at least try, right?
> As a minimum requirement I would expect this to be marked by
>
> struct device_driver {
> (...)
> /* This module absolutely cannot be unbound */
> .suppress_bind_attrs = true;
> };
No, that's not what bind/unbind is really for. That's a per-subsystem
choice as to if you want to allow devices to be added/removed from
drivers at runtime. It has nothing to do with module load/unload.
> So that noone would be able to try to unbind this (could even be an
> attack vector!)
>
> What is our broader reasoning when it comes to this? (I might have
> missed some mail thread here.)
Android is just finally pushing vendors to get their code upstream,
which is a good thing to see. And building things as a module is an
even better thing as now it is finally allowing arm64 systems to be
built to support more than one specific hardware platform at runtime.
So moving drivers to modules is good. If a module can be removed, even
better, but developers should not be lazy and just flat out not try at
all to make their code unloadable if at all possible.
Does that help?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH][next] pinctrl: lpc18xx: Use fallthrough pseudo-keyword
From: Linus Walleij @ 2020-07-17 12:06 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Vladimir Zapolskiy, open list:GPIO SUBSYSTEM, Linux ARM,
linux-kernel@vger.kernel.org, Gustavo A. R. Silva
In-Reply-To: <20200716212109.GA17525@embeddedor>
On Thu, Jul 16, 2020 at 11:15 PM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
> Replace the existing /* fall through */ comments and its variants with
> the new pseudo-keyword macro fallthrough[1].
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Linus Walleij @ 2020-07-17 12:01 UTC (permalink / raw)
To: Anson Huang, Greg KH, John Stultz
Cc: Russell King, Shawn Guo, Sascha Hauer, Sascha Hauer,
Fabio Estevam, Catalin Marinas, Will Deacon, Bartosz Golaszewski,
oleksandr.suvorov@toradex.com, Adam Ford, Andreas Kemnade,
hverkuil-cisco@xs4all.nl, Bjorn Andersson, Leo Li, Vinod Koul,
Geert Uytterhoeven, Olof Johansson, Linux ARM,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx, Jon Corbet
In-Reply-To: <DB3PR0402MB39167A4BB808A0679257DEF9F57F0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Greg, John,
we need some guidance here. See below.
On Thu, Jul 16, 2020 at 4:38 PM Anson Huang <anson.huang@nxp.com> wrote:
> [Me]
> > On Wed, Jul 15, 2020 at 4:44 AM Anson Huang <anson.huang@nxp.com>
> > > I tried to replace the subsys_initcall() with
> > > module_platform_driver(), but met issue about "
> > > register_syscore_ops(&mxc_gpio_syscore_ops);" which is called in
> > > gpio_mxc_init() function, this function should be called ONLY once,
> > > moving it to .probe function is NOT working, so we may need to keep the
> > > gpio_mxc_init(), that is another reason that we may need to keep
> > > subsys_initcall()?
> >
> > This looks a bit dangerous to keep like this while allowing this code to be used
> > from a module.
> >
> > What happens if you insmod and rmmod this a few times, really?
> > How is this tested?
> >
> > This is not really modularized if that isn't working, just that modprobing once
> > works isn't real modularization IMO, it seems more like a quick and dirty way
> > to get Androids GKI somewhat working with the module while not properly
> > making the module a module.
> >
> > You need input from the driver maintainers on how to handle this.
>
> As far as I know, some general/critical modules are NOT supporting rmmod, like
> clk, pinctrl, gpio etc., and I am NOT sure whether Android GKI need to support
> rmmod for these system-wide-used module, I will ask them for more detail about
> this.
>
> The requirement I received is to support loadable module, but so far no hard requirement
> to support module remove for gpio driver, so, is it OK to add it step by step, and this patch
> series ONLY to support module build and one time modprobe?
While I am a big fan of the Android GKI initiative this needs to be aligned
with the Linux core maintainers, so let's ask Greg. I am also paging
John Stultz on this: he is close to this action.
They both know the Android people very well.
So there is a rationale like this going on: in order to achieve GKI goals
and have as much as possible of the Linux kernel stashed into loadable
kernel modules, it has been elevated to modus operandi amongst
the developers pushing this change that it is OK to pile up a load of
modules that cannot ever be unloaded.
This is IIUC regardless of whether all consumers of the module are
actually gone: it would be OK to say make it impossible to rmmod
a clk driver even of zero clocks from that driver is in use. So it is not
dependency-graph problem, it is a "load once, never remove" approach.
This rationale puts me as subsystem maintainer in an unpleasant spot:
it is really hard to tell case-to-case whether that change really is a
technical advantage for the kernel per se or whether it is done for the
greater ecosystem of Android.
Often I would say it makes it possible to build a smaller kernel vmlinux
so OK that is an advantage. On the other hand I have an inkling that I
should be pushing developers to make sure that rmmod works.
As a minimum requirement I would expect this to be marked by
struct device_driver {
(...)
/* This module absolutely cannot be unbound */
.suppress_bind_attrs = true;
};
So that noone would be able to try to unbind this (could even be an
attack vector!)
What is our broader reasoning when it comes to this? (I might have
missed some mail thread here.)
Yours,
Linus Walleij
^ permalink raw reply
* Re: [GIT PULL] pinctrl: sh-pfc: Updates for v5.9 (take two)
From: Linus Walleij @ 2020-07-17 11:38 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: open list:GPIO SUBSYSTEM, Linux-Renesas
In-Reply-To: <20200717100944.15966-1-geert+renesas@glider.be>
On Fri, Jul 17, 2020 at 12:58 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The following changes since commit b2fc9b4eb1d79c03fd78e50b810c2ea27178e1e3:
>
> pinctrl: sh-pfc: r8a77970: Add RPC pins, groups, and functions (2020-06-22 16:58:23 +0200)
>
> are available in the Git repository at:
Thanks, pulled in to my devel branch.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] pinctrl: rockchip: Replace HTTP links with HTTPS ones
From: Linus Walleij @ 2020-07-17 11:31 UTC (permalink / raw)
To: Alexander A. Klimov
Cc: Heiko Stübner, open list:GPIO SUBSYSTEM, Linux ARM,
open list:ARM/Rockchip SoC..., linux-kernel@vger.kernel.org
In-Reply-To: <20200713183541.36963-1-grandmaster@al2klimov.de>
On Mon, Jul 13, 2020 at 8:35 PM Alexander A. Klimov
<grandmaster@al2klimov.de> wrote:
> Rationale:
> Reduces attack surface on kernel devs opening the links for MITM
> as HTTPS traffic is much harder to manipulate.
>
> Deterministic algorithm:
> For each file:
> If not .svg:
> For each line:
> If doesn't contain `\bxmlns\b`:
> For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
> If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`:
> If both the HTTP and HTTPS versions
> return 200 OK and serve the same content:
> Replace HTTP with HTTPS.
>
> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] gpio: crystalcove: Use irqchip template
From: Linus Walleij @ 2020-07-17 11:25 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko,
Kuppuswamy Sathyanarayanan, Hans de Goede
This makes the driver use the irqchip template to assign
properties to the gpio_irq_chip instead of using the
explicit calls to gpiochip_irqchip_add_nested() and
gpiochip_set_nested_irqchip(). The irqchip is instead
added while adding the gpiochip.
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Intel folks and Hans: I hope someone can test this, I'm
a bit uncertain if IRQs could fire before registering
the chip and if we need a hw_init() in this driver to cope.
---
drivers/gpio/gpio-crystalcove.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c
index 14d1f4c933b6..424a00ba1c97 100644
--- a/drivers/gpio/gpio-crystalcove.c
+++ b/drivers/gpio/gpio-crystalcove.c
@@ -330,6 +330,7 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
int retval;
struct device *dev = pdev->dev.parent;
struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
+ struct gpio_irq_chip *girq;
if (irq < 0)
return irq;
@@ -353,14 +354,15 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
cg->chip.dbg_show = crystalcove_gpio_dbg_show;
cg->regmap = pmic->regmap;
- retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg);
- if (retval) {
- dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
- return retval;
- }
-
- gpiochip_irqchip_add_nested(&cg->chip, &crystalcove_irqchip, 0,
- handle_simple_irq, IRQ_TYPE_NONE);
+ girq = &ch->chip.irq;
+ girq->chip = &crystalcove_irqchip;
+ /* This will let us handle the parent IRQ in the driver */
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->parents = NULL;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_simple_irq;
+ girq->threaded = true;
retval = request_threaded_irq(irq, NULL, crystalcove_gpio_irq_handler,
IRQF_ONESHOT, KBUILD_MODNAME, cg);
@@ -370,7 +372,11 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
return retval;
}
- gpiochip_set_nested_irqchip(&cg->chip, &crystalcove_irqchip, irq);
+ retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg);
+ if (retval) {
+ dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
+ return retval;
+ }
return 0;
}
--
2.26.2
^ permalink raw reply related
* [GIT PULL] pinctrl: sh-pfc: Updates for v5.9 (take two)
From: Geert Uytterhoeven @ 2020-07-17 10:09 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-renesas-soc, Geert Uytterhoeven
Hi Linus,
The following changes since commit b2fc9b4eb1d79c03fd78e50b810c2ea27178e1e3:
pinctrl: sh-pfc: r8a77970: Add RPC pins, groups, and functions (2020-06-22 16:58:23 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git tags/sh-pfc-for-v5.9-tag2
for you to fetch changes up to 4d0e62679f17b8bde01aa9995233b5b9ca05ab7f:
dt-bindings: pinctrl: renesas,rza2-pinctrl: Convert to json-schema (2020-07-16 09:57:50 +0200)
----------------------------------------------------------------
pinctrl: sh-pfc: Updates for v5.9 (take two)
- Add support for the new RZ/G2H (R8A774E1) SoC,
- One more conversion of DT bindings to json-schema,
- Fix RZ/A1 kerneldoc.
Thanks for pulling!
----------------------------------------------------------------
Geert Uytterhoeven (1):
dt-bindings: pinctrl: renesas,rza2-pinctrl: Convert to json-schema
Lad Prabhakar (1):
pinctrl: sh-pfc: pfc-r8a77951: Add R8A774E1 PFC support
Lee Jones (1):
pinctrl: rza1: Demote some kerneldoc headers and fix others
Marian-Cristian Rotariu (1):
dt-bindings: pinctrl: sh-pfc: Document r8a774e1 PFC support
.../bindings/pinctrl/renesas,pfc-pinctrl.txt | 1 +
.../bindings/pinctrl/renesas,rza2-pinctrl.txt | 87 --
.../bindings/pinctrl/renesas,rza2-pinctrl.yaml | 100 +++
drivers/pinctrl/pinctrl-rza1.c | 24 +-
drivers/pinctrl/sh-pfc/Kconfig | 4 +
drivers/pinctrl/sh-pfc/Makefile | 1 +
drivers/pinctrl/sh-pfc/core.c | 6 +
drivers/pinctrl/sh-pfc/pfc-r8a77951.c | 877 +++++++++++----------
drivers/pinctrl/sh-pfc/sh_pfc.h | 1 +
9 files changed, 587 insertions(+), 514 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza2-pinctrl.txt
create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza2-pinctrl.yaml
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* RE: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Anson Huang @ 2020-07-17 9:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Baluta, Aisheng Dong, festevam@gmail.com,
shawnguo@kernel.org, stefan@agner.ch, kernel@pengutronix.de,
linus.walleij@linaro.org, s.hauer@pengutronix.de,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <CAK8P3a256uR0zZn9ew9UoDqP51MdA4emCfMoZuPW6n9MqD5vPQ@mail.gmail.com>
Hi, Arnd
> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as
> module
>
> On Fri, Jul 17, 2020 at 11:24 AM Anson Huang <anson.huang@nxp.com>
> wrote:
> > > Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl
> > > driver as module
> > > +MODULE_AUTHOR("Dong Aisheng<aisheng.dong@nxp.com>");
> > > +MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
> > > +MODULE_LICENSE("GPL v2");
> > >
> > >
> > > This can be in a separate patch.
> >
> > I don't understand, without adding module license, changing the SCU
> > pinctrl driver to "tristate", when building with =M, the build will
> > have warning as below, so I think it does NOT make sense to split it to 2
> patches.
> >
> > CC [M] drivers/pinctrl/freescale/pinctrl-scu.o
> > MODPOST Module.symvers
> > WARNING: modpost: missing MODULE_LICENSE() in
> drivers/pinctrl/freescale/pinctrl-scu.o
> > LD [M] drivers/pinctrl/freescale/pinctrl-scu.ko
>
> I agree it would be clearer to do it as separate patches, but you then have to
> be careful about the order to avoid the problem you mention.
>
> A clear indication that it may be sensible to split up the patch is that your
> changelog has a list of five items in it, which are mostly doing different things.
> The ideal way to split up a patch series is to have each patch with a changelog
> that has to explain exactly one thing, and makes it obvious how each changed
> line corresponds to the description, but never explain the same thing in more
> than one patch (i.e. you combine patches that do the same thing in multiple
> files).
>
> In this case, a good split may be:
>
> patch 1:
> - Use function callbacks for SCU related functions in pinctrl-imx.c
> in order to support the scenario of PINCTRL_IMX is built in
> while PINCTRL_IMX_SCU is built as module;
> - All drivers using SCU pinctrl driver need to initialize the
> SCU related function callback;
>
> patch 2:
> - Export SCU related functions and use "IS_ENABLED" instead of
> "ifdef" to support SCU pinctrl driver user and itself to be
> built as module;
> - Change PINCTR_IMX_SCU to tristate;
> - Add module author, description and license.
>
> and then rewrite the description to not have a bulleted list.
>
> That said, I don't think it is critical here, and I would not have complained
> about the version you sent.
>
> If you end up changing the patch, I think you can actually drop the "#if
> IS_ENABLED()" check entirely, as the functions are now always assumed to be
> available, and we don't #ifdef declarations when there is no #else path
> otherwise.
>
Thanks for the good suggestion, if there is other comment need a V2, or maintainer
thinks it is better to split it following your guide, I will send V2 following your guide.
Thanks,
Anson
^ permalink raw reply
* Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Arnd Bergmann @ 2020-07-17 9:45 UTC (permalink / raw)
To: Anson Huang
Cc: Daniel Baluta, Aisheng Dong, festevam@gmail.com,
shawnguo@kernel.org, stefan@agner.ch, kernel@pengutronix.de,
linus.walleij@linaro.org, s.hauer@pengutronix.de,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <DB3PR0402MB3916A84BEF5EEC327EE35180F57C0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
On Fri, Jul 17, 2020 at 11:24 AM Anson Huang <anson.huang@nxp.com> wrote:
> > Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
> > +MODULE_AUTHOR("Dong Aisheng<aisheng.dong@nxp.com>");
> > +MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
> > +MODULE_LICENSE("GPL v2");
> >
> >
> > This can be in a separate patch.
>
> I don't understand, without adding module license, changing the SCU pinctrl driver
> to "tristate", when building with =M, the build will have warning as below, so I think
> it does NOT make sense to split it to 2 patches.
>
> CC [M] drivers/pinctrl/freescale/pinctrl-scu.o
> MODPOST Module.symvers
> WARNING: modpost: missing MODULE_LICENSE() in drivers/pinctrl/freescale/pinctrl-scu.o
> LD [M] drivers/pinctrl/freescale/pinctrl-scu.ko
I agree it would be clearer to do it as separate patches, but you then
have to be careful about the order to avoid the problem you mention.
A clear indication that it may be sensible to split up the patch is that
your changelog has a list of five items in it, which are mostly doing
different things. The ideal way to split up a patch series is to have
each patch with a changelog that has to explain exactly one thing,
and makes it obvious how each changed line corresponds to the
description, but never explain the same thing in more than one patch
(i.e. you combine patches that do the same thing in multiple files).
In this case, a good split may be:
patch 1:
- Use function callbacks for SCU related functions in pinctrl-imx.c
in order to support the scenario of PINCTRL_IMX is built in
while PINCTRL_IMX_SCU is built as module;
- All drivers using SCU pinctrl driver need to initialize the
SCU related function callback;
patch 2:
- Export SCU related functions and use "IS_ENABLED" instead of
"ifdef" to support SCU pinctrl driver user and itself to be
built as module;
- Change PINCTR_IMX_SCU to tristate;
- Add module author, description and license.
and then rewrite the description to not have a bulleted list.
That said, I don't think it is critical here, and I would not have
complained about the version you sent.
If you end up changing the patch, I think you can actually drop
the "#if IS_ENABLED()" check entirely, as the functions are
now always assumed to be available, and we don't #ifdef
declarations when there is no #else path otherwise.
Arnd
^ permalink raw reply
* RE: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Anson Huang @ 2020-07-17 9:22 UTC (permalink / raw)
To: Daniel Baluta, Aisheng Dong, festevam@gmail.com,
shawnguo@kernel.org, stefan@agner.ch, kernel@pengutronix.de,
linus.walleij@linaro.org, s.hauer@pengutronix.de,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: dl-linux-imx
In-Reply-To: <c27109aa-280f-db0d-980b-fbd735bab0a6@nxp.com>
Hi, Daniel
> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as
> module
>
> On 7/17/20 2:44 AM, Anson Huang wrote:
> > Hi, Daniel
> >
> >
> >> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl
> >> driver as module
> >>
> >> On 7/16/20 6:21 PM, Anson Huang wrote:
> >>> Hi, Daniel
> >>>
> >>>
> >>>> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl
> >>>> driver as module
> >>>>
> >>>> Hi Anson,
> >>>>
> >>>> Few comments inline:
> >>>>
> >>>> On 7/16/20 6:06 PM, Anson Huang wrote:
> >>>>> To support building i.MX SCU pinctrl driver as module, below
> >>>>> things need to
> >>>> be changed:
> >>>>> - Export SCU related functions and use "IS_ENABLED" instead of
> >>>>> "ifdef" to support SCU pinctrl driver user and itself to be
> >>>>> built as module;
> >>>>> - Use function callbacks for SCU related functions in
> pinctrl-imx.c
> >>>>> in order to support the scenario of PINCTRL_IMX is built in
> >>>>> while PINCTRL_IMX_SCU is built as module;
> >>>>> - All drivers using SCU pinctrl driver need to initialize the
> >>>>> SCU related function callback;
> >>>>> - Change PINCTR_IMX_SCU to tristate;
> >>>>> - Add module author, description and license.
> >>>>>
> >>>>> With above changes, i.MX SCU pinctrl driver can be built as module.
> >>>> There are a lot of changes here. I think it would be better to try
> >>>> to split them
> >>>>
> >>>> per functionality. One functional change per patch.
> >>> Actually, I ever tried to split them, but the function will be broken.
> >>> All the changes are just to support the module build. If split them,
> >>> the bisect will have pinctrl build or function broken.
> >> Hi Anson,
> >>
> >>
> >> I see your point and I know that this is a very hard task to get it
> >> right from
> >>
> >> the first patches.
> >>
> >> But let me suggest at least that:
> >>
> >> - changes in drivers/pinctrl/freescale/pinctrl-imx.c (include file
> >> and MODULE_ macros should go to a separate patch).
> > You meant in patch #2, the changes in Kconfig and the changes in .c
> > file should be split to 2 patches?
>
>
> No. I mean look at path #1.
>
> The section above should be placed in a separate patch.
>
> static const struct of_device_id imx8qxp_pinctrl_of_match[] = { diff --git
> a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
> index 9df45d3..59b5f8a 100644
> --- a/drivers/pinctrl/freescale/pinctrl-scu.c
> +++ b/drivers/pinctrl/freescale/pinctrl-scu.c
> @@ -7,6 +7,7 @@
>
> #include <linux/err.h>
> #include <linux/firmware/imx/sci.h>
> +#include <linux/module.h>
> #include <linux/of_address.h>
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/platform_device.h>
> @@ -123,3 +124,7 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl
> *ipctl,
> pin_scu->mux_mode, pin_scu->config);
> }
> EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
> +
> +MODULE_AUTHOR("Dong Aisheng<aisheng.dong@nxp.com>");
> +MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
> +MODULE_LICENSE("GPL v2");
>
>
> This can be in a separate patch.
I don't understand, without adding module license, changing the SCU pinctrl driver
to "tristate", when building with =M, the build will have warning as below, so I think
it does NOT make sense to split it to 2 patches.
CC [M] drivers/pinctrl/freescale/pinctrl-scu.o
MODPOST Module.symvers
WARNING: modpost: missing MODULE_LICENSE() in drivers/pinctrl/freescale/pinctrl-scu.o
LD [M] drivers/pinctrl/freescale/pinctrl-scu.ko
Thanks,
Anson
^ permalink raw reply
* Re: [PATCH v5 02/13] mfd: add simple regmap based I2C driver
From: Lee Jones @ 2020-07-17 9:06 UTC (permalink / raw)
To: Michael Walle
Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
linux-watchdog, linux-arm-kernel, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Mark Brown, Greg Kroah-Hartman, Andy Shevchenko
In-Reply-To: <20200706175353.16404-3-michael@walle.cc>
On Mon, 06 Jul 2020, Michael Walle wrote:
> There are I2C devices which contain several different functions but
> doesn't require any special access functions. For these kind of drivers
> an I2C regmap should be enough.
>
> Create an I2C driver which creates an I2C regmap and enumerates its
> children. If a device wants to use this as its MFD core driver, it has
> to add an individual compatible string. It may provide its own regmap
> configuration.
>
> Subdevices can use dev_get_regmap() on the parent to get their regmap
> instance.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> Changes since v4:
> - new patch. Lee, please bear with me. I didn't want to delay the
> new version (where a lot of remarks on the other patches were
> addressed) even more, just because we haven't figured out how
> to deal with the MFD part. So for now, I've included this one.
>
> drivers/mfd/Kconfig | 9 +++++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/simple-mfd-i2c.c | 50 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 60 insertions(+)
> create mode 100644 drivers/mfd/simple-mfd-i2c.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 33df0837ab41..f1536a710aca 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1162,6 +1162,15 @@ config MFD_SI476X_CORE
> To compile this driver as a module, choose M here: the
> module will be called si476x-core.
>
> +config MFD_SIMPLE_MFD_I2C
> + tristate "Simple regmap based I2C devices"
Doesn't look like tristate to me.
Haven't you made this builtin only?
> + depends on I2C
> + select MFD_CORE
Why?
> + select REGMAP_I2C
> + help
> + This is a consolidated driver for all MFD devices which are
> + basically just a regmap bus driver.
Please expand on this. I think it deserves greater explanation.
> config MFD_SM501
> tristate "Silicon Motion SM501"
> depends on HAS_DMA
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v5 02/13] mfd: add simple regmap based I2C driver
From: Lee Jones @ 2020-07-17 9:04 UTC (permalink / raw)
To: Michael Walle
Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
linux-watchdog, linux-arm-kernel, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Mark Brown, Greg Kroah-Hartman, Andy Shevchenko
In-Reply-To: <20200706175353.16404-3-michael@walle.cc>
On Mon, 06 Jul 2020, Michael Walle wrote:
> There are I2C devices which contain several different functions but
> doesn't require any special access functions. For these kind of drivers
> an I2C regmap should be enough.
>
> Create an I2C driver which creates an I2C regmap and enumerates its
> children. If a device wants to use this as its MFD core driver, it has
> to add an individual compatible string. It may provide its own regmap
> configuration.
>
> Subdevices can use dev_get_regmap() on the parent to get their regmap
> instance.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> Changes since v4:
> - new patch. Lee, please bear with me. I didn't want to delay the
> new version (where a lot of remarks on the other patches were
> addressed) even more, just because we haven't figured out how
> to deal with the MFD part. So for now, I've included this one.
>
> drivers/mfd/Kconfig | 9 +++++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/simple-mfd-i2c.c | 50 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 60 insertions(+)
> create mode 100644 drivers/mfd/simple-mfd-i2c.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 33df0837ab41..f1536a710aca 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1162,6 +1162,15 @@ config MFD_SI476X_CORE
> To compile this driver as a module, choose M here: the
> module will be called si476x-core.
>
> +config MFD_SIMPLE_MFD_I2C
> + tristate "Simple regmap based I2C devices"
> + depends on I2C
> + select MFD_CORE
> + select REGMAP_I2C
> + help
> + This is a consolidated driver for all MFD devices which are
> + basically just a regmap bus driver.
> +
> config MFD_SM501
> tristate "Silicon Motion SM501"
> depends on HAS_DMA
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index a60e5f835283..78d24a3e7c9e 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -264,3 +264,4 @@ obj-$(CONFIG_MFD_STMFX) += stmfx.o
> obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o
>
> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
> +obj-$(CONFIG_MFD_SIMPLE_MFD_I2C) += simple-mfd-i2c.o
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> new file mode 100644
> index 000000000000..1fdca89964b1
> --- /dev/null
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
I'm pretty sure you do not require all of these headers.
> +struct simple_mfd_i2c_config {
> + const struct regmap_config *regmap_config;
> +};
No need for this yet I feel.
Let's keep it as simple as possible.
> +static const struct regmap_config simple_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> +{
> + const struct regmap_config *regmap_config = &simple_regmap_config;
> + const struct simple_mfd_i2c_config *config;
> + struct regmap *regmap;
> +
> + config = device_get_match_data(&i2c->dev);
Have this return regmap_config.
> + if (config && config->regmap_config)
> + regmap_config = config->regmap_config;
> +
> + regmap = devm_regmap_init_i2c(i2c, regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + return devm_of_platform_populate(&i2c->dev);
> +}
> +
> +static const struct of_device_id simple_mfd_i2c_of_match[] = {
> + {}
> +};
> +
> +static struct i2c_driver simple_mfd_i2c_driver = {
> + .probe_new = simple_mfd_i2c_probe,
> + .driver = {
> + .name = "simple-mfd-i2c",
> + .of_match_table = simple_mfd_i2c_of_match,
> + },
> +};
> +builtin_i2c_driver(simple_mfd_i2c_driver);
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Daniel Baluta @ 2020-07-17 8:39 UTC (permalink / raw)
To: Anson Huang, Aisheng Dong, festevam@gmail.com,
shawnguo@kernel.org, stefan@agner.ch, kernel@pengutronix.de,
linus.walleij@linaro.org, s.hauer@pengutronix.de,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: dl-linux-imx
In-Reply-To: <DB3PR0402MB3916AAEE01257A7F9A25A657F57F0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
On 7/17/20 2:44 AM, Anson Huang wrote:
> Hi, Daniel
>
>
>> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as
>> module
>>
>> On 7/16/20 6:21 PM, Anson Huang wrote:
>>> Hi, Daniel
>>>
>>>
>>>> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl
>>>> driver as module
>>>>
>>>> Hi Anson,
>>>>
>>>> Few comments inline:
>>>>
>>>> On 7/16/20 6:06 PM, Anson Huang wrote:
>>>>> To support building i.MX SCU pinctrl driver as module, below things
>>>>> need to
>>>> be changed:
>>>>> - Export SCU related functions and use "IS_ENABLED" instead of
>>>>> "ifdef" to support SCU pinctrl driver user and itself to be
>>>>> built as module;
>>>>> - Use function callbacks for SCU related functions in pinctrl-imx.c
>>>>> in order to support the scenario of PINCTRL_IMX is built in
>>>>> while PINCTRL_IMX_SCU is built as module;
>>>>> - All drivers using SCU pinctrl driver need to initialize the
>>>>> SCU related function callback;
>>>>> - Change PINCTR_IMX_SCU to tristate;
>>>>> - Add module author, description and license.
>>>>>
>>>>> With above changes, i.MX SCU pinctrl driver can be built as module.
>>>> There are a lot of changes here. I think it would be better to try to
>>>> split them
>>>>
>>>> per functionality. One functional change per patch.
>>> Actually, I ever tried to split them, but the function will be broken.
>>> All the changes are just to support the module build. If split them,
>>> the bisect will have pinctrl build or function broken.
>> Hi Anson,
>>
>>
>> I see your point and I know that this is a very hard task to get it right from
>>
>> the first patches.
>>
>> But let me suggest at least that:
>>
>> - changes in drivers/pinctrl/freescale/pinctrl-imx.c (include file and
>> MODULE_ macros should go to a separate patch).
> You meant in patch #2, the changes in Kconfig and the changes in .c file should
> be split to 2 patches?
No. I mean look at path #1.
The section above should be placed in a separate patch.
static const struct of_device_id imx8qxp_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
index 9df45d3..59b5f8a 100644
--- a/drivers/pinctrl/freescale/pinctrl-scu.c
+++ b/drivers/pinctrl/freescale/pinctrl-scu.c
@@ -7,6 +7,7 @@
#include <linux/err.h>
#include <linux/firmware/imx/sci.h>
+#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/platform_device.h>
@@ -123,3 +124,7 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
pin_scu->mux_mode, pin_scu->config);
}
EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
+
+MODULE_AUTHOR("Dong Aisheng<aisheng.dong@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
+MODULE_LICENSE("GPL v2");
This can be in a separate patch.
^ permalink raw reply related
* Re: [PATCH v5 12/13] arm64: dts: freescale: sl28: enable LED support
From: Pavel Machek @ 2020-07-17 8:36 UTC (permalink / raw)
To: Michael Walle
Cc: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
linux-watchdog, linux-arm-kernel, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
Lee Jones, Thierry Reding, Uwe Kleine-K??nig, Wim Van Sebroeck,
Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Mark Brown, Greg Kroah-Hartman, Andy Shevchenko
In-Reply-To: <20200706175353.16404-13-michael@walle.cc>
Hi!
> Now that we have support for GPIO lines of the SMARC connector, enable
> LED support on the KBox A-230-LS. There are two LEDs without fixed
> functions, one is yellow and one is green. Unfortunately, it is just one
> multi-color LED, thus while it is possible to enable both at the same
> time it is hard to tell the difference between "yellow only" and "yellow
> and green".
> + user_yellow {
> + label = "s1914:yellow:user";
> + gpios = <&sl28cpld_gpio0 0 0>;
> + };
> +
> + user_green {
> + label = "s1914:green:user";
> + gpios = <&sl28cpld_gpio1 3 0>;
> + };
This is not suitable label for such LEDs... there's zero chance userland will
know what to do with these.
Do they have some kind of "usual" function?
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* RE: [PATCH] gpio: adp5588: Use irqchip template
From: Hennerich, Michael @ 2020-07-17 8:33 UTC (permalink / raw)
To: Linus Walleij, linux-gpio@vger.kernel.org
Cc: Bartosz Golaszewski, Nikolaus Voss
In-Reply-To: <20200716150502.195821-1-linus.walleij@linaro.org>
> -----Original Message-----
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: Donnerstag, 16. Juli 2020 17:05
> To: linux-gpio@vger.kernel.org
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>; Linus Walleij
> <linus.walleij@linaro.org>; Nikolaus Voss <nv@vosn.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>
> Subject: [PATCH] gpio: adp5588: Use irqchip template
>
> This makes the driver use the irqchip template to assign properties to the
> gpio_irq_chip instead of using the explicit calls to
> gpiochip_irqchip_add_nested() and gpiochip_set_nested_irqchip(). The irqchip
> is instead added while adding the gpiochip.
>
> Cc: Nikolaus Voss <nv@vosn.de>
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
> drivers/gpio/gpio-adp5588.c | 39 +++++++++++++++++++++++--------------
> 1 file changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c index
> 49f423d7beba..f1e4ac90e7d3 100644
> --- a/drivers/gpio/gpio-adp5588.c
> +++ b/drivers/gpio/gpio-adp5588.c
> @@ -272,13 +272,24 @@ static irqreturn_t adp5588_irq_handler(int irq, void
> *devid)
> return IRQ_HANDLED;
> }
>
> +
> +static int adp5588_irq_init_hw(struct gpio_chip *gc) {
> + struct adp5588_gpio *dev = gpiochip_get_data(gc);
> + /* Enable IRQs after registering chip */
> + adp5588_gpio_write(dev->client, CFG,
> + ADP5588_AUTO_INC | ADP5588_INT_CFG |
> ADP5588_KE_IEN);
> +
> + return 0;
> +}
> +
> static int adp5588_irq_setup(struct adp5588_gpio *dev) {
> struct i2c_client *client = dev->client;
> int ret;
> struct adp5588_gpio_platform_data *pdata =
> dev_get_platdata(&client->dev);
> - int irq_base = pdata ? pdata->irq_base : 0;
> + struct gpio_irq_chip *girq;
>
> adp5588_gpio_write(client, CFG, ADP5588_AUTO_INC);
> adp5588_gpio_write(client, INT_STAT, -1); /* status is W1C */ @@ -
> 294,21 +305,19 @@ static int adp5588_irq_setup(struct adp5588_gpio *dev)
> client->irq);
> return ret;
> }
> - ret = gpiochip_irqchip_add_nested(&dev->gpio_chip,
> - &adp5588_irq_chip, irq_base,
> - handle_simple_irq,
> - IRQ_TYPE_NONE);
> - if (ret) {
> - dev_err(&client->dev,
> - "could not connect irqchip to gpiochip\n");
> - return ret;
> - }
> - gpiochip_set_nested_irqchip(&dev->gpio_chip,
> - &adp5588_irq_chip,
> - client->irq);
>
> - adp5588_gpio_write(client, CFG,
> - ADP5588_AUTO_INC | ADP5588_INT_CFG |
> ADP5588_KE_IEN);
> + /* This will be registered in the call to devm_gpiochip_add_data() */
> + girq = &dev->gpio_chip.irq;
> + girq->chip = &adp5588_irq_chip;
> + /* This will let us handle the parent IRQ in the driver */
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->first = pdata ? pdata->irq_base : 0;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_simple_irq;
> + girq->init_hw = adp5588_irq_init_hw;
> + girq->threaded = true;
>
> return 0;
> }
> --
> 2.26.2
^ permalink raw reply
* Re: [PATCH v4 00/16] Allwinner A100 Initial support
From: Frank Lee @ 2020-07-17 7:38 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Frank Lee, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
Michael Turquette, Stephen Boyd, gregory.clement, Thomas Gleixner,
jason, Marc Zyngier, Srini Kandagatla, Linus Walleij,
Vasily Khoruzhick, Zhang Rui, Amit Kucheria, Lee Jones, p.zabel,
clabbe, Icenowy Zheng, Ondřej Jirman, stefan, bage,
devicetree, Linux ARM, Linux Kernel Mailing List, linux-clk,
linux-i2c, linux-gpio, Linux PM
In-Reply-To: <ffd5eead-571c-6548-0527-1e685ec869ef@linaro.org>
On Fri, Jul 17, 2020 at 12:28 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 14/07/2020 08:55, Frank Lee wrote:
> > From: Yangtao Li <frank@allwinnertech.com>
>
> Do you expect me to pick patches 7,8,9 or ack them ?
>
Please pick it.
Thx,
Yangtao
^ 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