Re: [PATCH v2 1/2] dt-bindings: pinctrl: qcom,pmic-gpio: Allow gpio-hog nodes
On Tue, Apr 9, 2024 at 8:36 PM Luca Weiss wrote: > Allow specifying a GPIO hog, as already used on > qcom-msm8974-lge-nexus5-hammerhead.dts. > > Signed-off-by: Luca Weiss This patch applied to the pinctrl tree! Yours, Linus Walleij
Re: [PATCH 64/64] i2c: reword i2c_algorithm in drivers according to newest specification
On Fri, Mar 22, 2024 at 2:27 PM Wolfram Sang wrote: > Match the wording in i2c_algorithm in I2C drivers wrt. the newest I2C > v7, SMBus 3.2, I3C specifications and replace "master/slave" with more > appropriate terms. For some drivers, this means no more conversions are > needed. For the others more work needs to be done but this will be > performed incrementally along with API changes/improvements. All these > changes here are simple search/replace results. > > Signed-off-by: Wolfram Sang Acked-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 1/4] asm-generic/page.h: apply page shift to PFN instead of VA in pfn_to_virt
On Wed, Jan 31, 2024 at 7:27 AM Yan Zhao wrote:
> Apply the page shift to PFN to get physical address for final VA.
> The macro __va should take physical address instead of PFN as input.
>
> Fixes: 2d78057f0dd4 ("asm-generic/page.h: Make pfn accessors static inlines")
> Signed-off-by: Yan Zhao
My bug, obviously. :(
Reviewed-by: Linus Walleij
I thought this was already applied with the other fixes, but maybe it
was missed?
Yours,
Linus Walleij
Re: [PATCH] mm: Remove broken pfn_to_virt() on arch csky/hexagon/openrisc
On Fri, Feb 2, 2024 at 3:35 PM Yan Zhao wrote: > Remove the broken pfn_to_virt() on architectures csky/hexagon/openrisc. > > The pfn_to_virt() on those architectures takes PFN instead of PA as the > input to macro __va(), with PAGE_SHIFT applying to the converted VA, which > is not right, especially when there's a non-zero offset in __va(). [1] > > The broken pfn_to_virt() was noticed when checking how page_to_virt() is > unfolded on x86. It turns out that the one in linux/mm.h instead of in > asm-generic/page.h is compiled for x86. However, page_to_virt() in > asm-generic/page.h is found out expanding to pfn_to_virt() with a bug > described above. The pfn_to_virt() is found out not right as well on > architectures csky/hexagon/openrisc. > > Since there's no single caller on csky/hexagon/openrisc to pfn_to_virt() > and there are functions doing similar things as pfn_to_virt() -- > - pfn_to_kaddr() in asm/page.h for csky, > - page_to_virt() in asm/page.h for hexagon, and > - page_to_virt() in linux/mm.h for openrisc, > just delete the pfn_to_virt() on those architectures. > > The pfn_to_virt() in asm-generic/page.h is not touched in this patch as > it's referenced by page_to_virt() in that header while the whole header is > going to be removed as a whole due to no one including it. > > Link:https://lore.kernel.org/all/20240131055159.2506-1-yan.y.z...@intel.com > [1] > Cc: Linus Walleij > Suggested-by: Arnd Bergmann > Signed-off-by: Yan Zhao Thanks for making the kernel a better place! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 0/4] apply page shift to PFN instead of VA in pfn_to_virt
On Wed, Jan 31, 2024 at 7:25 AM Yan Zhao wrote: > This is a tiny fix to pfn_to_virt() for some platforms. > > The original implementaion of pfn_to_virt() takes PFN instead of PA as the > input to macro __va, with PAGE_SHIFT applying to the converted VA, which > is not right under most conditions, especially when there's an offset in > __va. Ooops that's right, I wonder why I got it wrong. Arithmetic made it not regress :/ Thank you so much for fixing this Yan! Reviewed-by: Linus Walleij Arnd: I think you can take most of them through the arch tree. Yours, Linus Walleij
Re: [PATCH v8 3/9] pinctrl: single: add marvell,pxa1908-padconf compatible
On Wed, Jan 10, 2024 at 8:04 PM Duje Mihanović wrote: > Add the "marvell,pxa1908-padconf" compatible to allow migrating to a > separate pinctrl driver later. > > Signed-off-by: Duje Mihanović Acked-by: Linus Walleij I guess you will merge all of this through the SoC tree? Yours, Linus Walleij
Re: [PATCH v8 2/9] dt-bindings: pinctrl: pinctrl-single: add marvell,pxa1908-padconf compatible
On Wed, Jan 10, 2024 at 8:04 PM Duje Mihanović wrote: > Add the "marvell,pxa1908-padconf" compatible to allow migrating to a > separate pinctrl driver later. > > Reviewed-by: Rob Herring > Signed-off-by: Duje Mihanović Acked-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v2 5/7] dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges
On Tue, Sep 19, 2023 at 2:46 PM Luca Weiss wrote: > Allow the gpio-reserved-ranges property on SC7280 TLMM. > > Acked-by: Linus Walleij > Acked-by: Krzysztof Kozlowski > Signed-off-by: Luca Weiss This patch 5/7 applied to the pinctrl tree! Yours, Linus Walleij
Re: [PATCH v2 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings
On Sat, Sep 23, 2023 at 3:14 PM Matti Lehtimäki wrote: > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > through MPM-connected pins. > > Signed-off-by: Matti Lehtimäki Both v2 patches applied! Yours, Linus Walleij
Re: [PATCH v2 0/3] Add blsp1_i2c6 and blsp1_uart2 to MSM8226 SoC
On Fri, Sep 22, 2023 at 6:56 PM Luca Weiss wrote: > Add the I2C bus and UART interface found on the MSM8226. For the I2C bus > we also first need to add the pinctrl function in the driver. > > Signed-off-by: Luca Weiss v2 looks fine and ACKs in place, so patches applied! Yours, Linus Walleij
Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error
On Sun, Apr 18, 2021 at 11:43 AM Jonathan Cameron wrote: > I'm still far from completely convinced that it is 'necessary' > to take the reference whilst going through this sequence because > there is nothing to kick off the suspend until we tell it to use > autosuspend. However, I appreciate (much like taking locks in > general in probe) that it makes it easy to see there is no race. One part is related to hierarchy, in the past if this device would sit on a I2C bus on an PCIE card, unless you take a reference (that escalate upwards) the I2C bus host or PCIE card may decide to go and runtime suspend, cutting communication with your device. Since 04f59143b571 the power state of I2C buses are decoupled from the clients and we decided that the I2C (as well as SPI hosts, separate commit) shall make sure PM resume themselves when transmitting messages to clients. So in this case, since it is an I2C device, we are probably fine without grabbing a reference. But this is not a general rule for any (non-slow) bus, so the idiomatic pattern to follow is better like this. Yours, Linus Walleij
Re: BUG: iio: mpu3050: Wrong temperature scale
On Mon, Apr 19, 2021 at 8:06 AM Dmitry Osipenko wrote: > The driver uses > (x+23000)/280 formula for the conversion of raw temperature value, which > gives 82C for x=0, thus apparently formula is wrong because x=5 > should give us ~25C. > > I tried to search for the datasheet with the formula, but couldn't find it. There is no public datasheet. I have never seen a non-public datasheet either. As the initial submission of the driver says: "This driver is based on information from the rough input driver in drivers/input/misc/mpu3050.c and the scratch misc driver posted by Nathan Royer in 2011. Some years have passed but this is finally a fully-fledged driver for this gyroscope. It was developed and tested on the Qualcomm APQ8060 Dragonboard." Nathans submission: https://lore.kernel.org/lkml/1309486707-1658-1-git-send-email-nro...@invensense.com/ (you find the threads at the bottom) This submission came from inside Invensense so it is the closest authoritative source we have. > Linus, will you be able to check whether the formula used by the driver > is correct? Thanks in advance. Sadly the code is the documentation when it comes to Invensense stuff, I am CC:ing Nathans Invensense address in the vain hope he is still working there and could help, also CC to Jean-Baptiste who was there last year and maybe can help out. I don't anymore remember exactly how I found this equation, but it wasn't from any datasheet. I vaguely remember browsing through some Android userspace sensor code. What I tend to do is dig around in old mobile phone Android trees, and there you sometimes find this information in different GPL code drops. I bet I got it from browsing some of those. Here is an example (Tegra): https://android.googlesource.com/kernel/tegra/+/dba2740d025c8e7e7e3c61d84a4f964d2c1c0ac9/drivers/misc/inv_mpu Worst case what one *can* do is to calibrate the scale, like put the device in a controlled environment of some two reasonably far apart temperatures and measure, assuming it is at least linear. Some professionals use controlled environment chambers for this. But I hope there is a better way. Yours, Linus Walleij
Re: [PATCH] dt-bindings: pinctrl: rockchip: add RK3568 SoC support
On Sat, Apr 10, 2021 at 10:45 PM Ezequiel Garcia wrote: > Add RK3568/RK3566 SoC support to pinctrl. > > Signed-off-by: Ezequiel Garcia Patch applied. Yours, Linus Walleij
Re: [PATCH v2 0/7] gpio-rockchip driver
On Sun, Apr 11, 2021 at 3:30 PM Peter Geis wrote: > Separate gpio driver from pinctrl driver, and support v2 controller. > > Tested on rk3566-quartz64 prototype board. > > Patch History: > V2 - Rebase to latest linux-next. > > Tested-by: Peter Geis This does not apply to the pin control tree, the problem with basing stuff on -next is that it sometimes does not apply to any development tree and now that happened (because of conflicts in the GPIO tree). You can either resend this based on the pinctrl "devel" branch: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel ... or you can wait until kernel v5.13-rc1 is out and then we can merge it, but it might even require rebasing after that. Yours, Linus Walleij
Re: [PATCH v2 1/3] dt-bindings: gpio: add YAML description for rockchip,gpio-bank
On Tue, Apr 13, 2021 at 12:36 AM Johan Jonker wrote: > Current dts files with "rockchip,gpio-bank" subnodes > are manually verified. In order to automate this process > the text that describes the compatible in rockchip,pinctrl.txt > is removed and converted to YAML in rockchip,gpio-bank.yaml. > > Signed-off-by: Johan Jonker > --- > Changed V2: > changed example gpio nodename Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v2] drivers: pinctrl: qcom: fix Kconfig dependency on GPIOLIB
On Wed, Apr 14, 2021 at 4:51 AM Julian Braha wrote: > When PINCTRL_MSM is enabled, and GPIOLIB is disabled, > Kbuild gives the following warning: > > WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP > Depends on [n]: GPIOLIB [=n] > Selected by [y]: > - PINCTRL_MSM [=y] && PINCTRL [=y] && (ARCH_QCOM || COMPILE_TEST [=y]) > > This is because PINCTRL_MSM selects GPIOLIB_IRQCHIP, > without selecting or depending on GPIOLIB, despite > GPIOLIB_IRQCHIP depending on GPIOLIB. Having PINCTRL_MSM > select GPIOLIB will cause a recursive dependency error. > > Signed-off-by: Julian Braha Patch applied. Yours, Linus Walleij
Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error
On Mon, Apr 12, 2021 at 12:16 PM Jonathan Cameron wrote: > An example would be the bmc150_magn driver which does exactly the > same call sequence as this one, but without the reference count increment > and decrement. Basically I want to know if there is a problem in > those other drivers that is being protected against here! The bmc150_magn driver does not look like I would have done it. That said, I think it works, because the only thing it is calling is bmc150_magn_set_power_mode() and that seems stateless, just poking some regmap bits. The state is tracked by the driver AFAICT and we don't need pm_runtime_get_noresume() because it doesn't really matter if the pm_runtime_suspend() callback gets called immediately or randomly out of sync with what we are doing from this point. I would anyways patch it like the gp2ap002 driver because it is easier to follow the code. Including using only runtime PM att setting SET_SYSTEM_SLEEP_PM_OPS() to the pm_runtime_force_suspend and pm_runtime_force_resume functions, everything just get so much easier when you use only one type of PM and not two orthogonal ones. drivers/iio/light/bh1780.c should be a good example of how to do it idiomatically because it was reviewed by Ulf Hansson who knows this runtime PM stuff better than me. A sequence like this: pm_runtime_get_noresume(>dev); pm_runtime_set_active(>dev); pm_runtime_enable(>dev); pm_runtime_set_autosuspend_delay(>dev, 5000); pm_runtime_use_autosuspend(>dev); pm_runtime_put(>dev); is very nice because you can clearly see that it will not race and after the last put() unless something happens the runtime suspend will kick in after 5000 ms. Likewise when disabling: pm_runtime_get_sync(>dev); pm_runtime_put_noidle(>dev); pm_runtime_disable(>dev); same thing: crystal clear there are no races, the device is definately runtime resumed and we can proceed to shut down resources explicitly after this point. If you then add: SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume) Now you have ordinary sleep PM for free. It will just force the same suspend/resume callbacks and they are guaranteed to be race free. This doesn't work for everyone but surprisingly often this is what you want. Yours, Linus Walleij
Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error
On Sun, Apr 11, 2021 at 5:07 PM Jonathan Cameron wrote: > On Wed, 7 Apr 2021 11:49:27 +0800 > Dinghao Liu wrote: > > > When devm_request_threaded_irq() fails, we should decrease the > > runtime PM counter to keep the counter balanced. But when > > iio_device_register() fails, we need not to decrease it because > > we have already decreased it before. > > Whilst agree with your assessment that the code is wrong, I'm not > totally sure why we need to do the pm_runtime_get_noresume() in > the first place. Why do we need to hold the reference for > the operations going on here? What can race against this that > might care about that reference count? pm_runtime_get_noresume() is increasing the runtime PM reference without calling the pm_runtime_resume() callback. It is often called in sequence like this: pm_runtime_get_noresume(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); This increases the reference, sets the device as active and enables runtime PM. The reason that probe() has activated resources such as enabling two regulators, and want to leave them on so that later on pm_runtime_suspend() will disable them, i.e. handover to runtime PM with the device in resumed state. I hope this is answering the question, not sure. Yours, Linus Walleij
Re: [PATCH] pinctrl: at91-pio4: Fix slew rate disablement
On Fri, Apr 9, 2021 at 10:25 AM Tudor Ambarus
wrote:
> The slew rate was enabled by default for each configuration of the
> pin. In case the pin had more than one configuration, even if
> we set the slew rate as disabled in the device tree, the next pin
> configuration would set again the slew rate enabled by default,
> overwriting the slew rate disablement.
> Instead of enabling the slew rate by default for each pin configuration,
> enable the slew rate by default just once per pin, regardless of the
> number of configurations. This way the slew rate disablement will also
> work for cases where pins have multiple configurations.
>
> Fixes: 440b144978ba ("pinctrl: at91-pio4: add support for slew-rate")
> Signed-off-by: Tudor Ambarus
Patch applied!
Yours,
Linus Walleij
Re: [PATCH] pinctrl: samsung: use 'int' for register masks in Exynos
On Thu, Apr 8, 2021 at 9:50 PM Krzysztof Kozlowski wrote: > The Special Function Registers on all Exynos SoC, including ARM64, are > 32-bit wide, so entire driver uses matching functions like readl() or > writel(). On 64-bit ARM using unsigned long for register masks: > 1. makes little sense as immediately after bitwise operation it will be >cast to 32-bit value when calling writel(), > 2. is actually error-prone because it might promote other operands to >64-bit. > > Addresses-Coverity: Unintentional integer overflow > Signed-off-by: Krzysztof Kozlowski (...) > Please apply it directly, I don't have any patches for Samsung pinctrl > in my tree. OK! Patch applied! Yours, Linus Walleij
Re: [PATCH v3] drivers: introduce and use WANT_DMA_CMA for soft dependencies on DMA_CMA
On Fri, Apr 9, 2021 at 1:20 PM David Hildenbrand wrote: > Random drivers should not override a user configuration of core knobs > (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA, > which depends on CMA, if possible; however, these drivers also have to > tolerate if DMA_CMA is not available/functioning, for example, if no CMA > area for DMA_CMA use has been setup via "cma=X". In the worst case, the > driver cannot do it's job properly in some configurations. Looks good to me. At least a lot better than what we have. Reviewed-by: Linus Walleij > Let's see if this approach is better for soft dependencies (and if we > actually have some hard dependencies in there). This is the follow-up > of > https://lkml.kernel.org/r/20210408092011.52763-1-da...@redhat.com > https://lkml.kernel.org/r/20210408100523.63356-1-da...@redhat.com You can just add these to the commit message with Link: when applying so people can easily find the discussion from the commit. > I was wondering if it would make sense in some drivers to warn if either > CONFIG_DMA_CMA is not available or if DRM_CMA has not been configured > properly - just to give people a heads up that something might more likely > go wrong; that would, however, be future work. I think the frameworks (DRM_*_CMA_HELPER) should pr_info something about it so the individual drivers don't have to sanity check their entire world. Yours, Linus Walleij
Re: [PATCHv2 35/38] dt-bindings: power: supply: ab8500: Convert to DT schema format
On Wed, Mar 17, 2021 at 2:51 PM Sebastian Reichel wrote: > Convert the binding to DT schema format. > > Note: The battery node does not have a compatible value and needs > to be described from the binding file for the PMIC. That has not > yet been converted, so I kept the information in plaintext for now. > > Signed-off-by: Sebastian Reichel Thanks for doing this Sebastian: Reviewed-by: Linus Walleij Long term I want to get rid of the "charging algorithm" device node, as can be easily seen that is not a real hardware device but just some mockery to get a platform device up and probed. It is actually a library. But for now we need to keep it around. Yours, Linus Walleij
Re: [GIT PULL] Apple M1 SoC platform bring-up for 5.13
On Thu, Apr 8, 2021 at 5:55 PM Hector Martin wrote: > Hi Arnd and all, > > Here's the final version of the M1 SoC bring-up series, based on > v4 which was reviewed here: > > https://lore.kernel.org/linux-arm-kernel/20210402090542.131194-1-mar...@marcan.st/T/#u Excellent work on this series Hector, thanks for working so hard on this! Yours, Linus Walleij
Re: New 'make dtbs_check W=1' warnings
On Thu, Apr 8, 2021 at 5:08 PM Arnd Bergmann wrote: > arch/arm/boot/dts/ste-href520-tvk.dt.yaml: accelerometer@19: > interrupts: [[18, 1], [19, 1]] is too long > arch/arm/boot/dts/ste-hrefprev60-tvk.dt.yaml: gyroscope@68: > interrupts-extended: [[22, 0, 1], [21, 31, 1]] is too long > arch/arm/boot/dts/ste-hrefv60plus-tvk.dt.yaml: gyroscope@68: > interrupts-extended: [[25, 0, 1], [24, 31, 1]] is too long > arch/arm/boot/dts/ste-hrefv60plus-tvk.dt.yaml: accelerometer@1c: > interrupts: [[18, 1], [19, 1]] is too long These warnings are all because the bindings in Documentation/devicetree/bindings/iio/st,st-sensors.yaml are slightly incorrect. Several sensors have more than 1 IRQ. I was working on a refined version of the bindings but got sidetracked. https://lore.kernel.org/linux-iio/20210104093343.2134410-1-linus.wall...@linaro.org/ I'll try to get to it. Yours, Linus Walleij
Re: [PATCH v2 1/2] dt-bindings: pinctrl: qcom-pmic-gpio: Add pm8008 support
On Thu, Apr 8, 2021 at 7:25 PM Guru Das Srinagesh wrote: > Add support for the 2 GPIOs present on Qualcomm Technologies, Inc. > PM8008. > > Acked-by: Bjorn Andersson > Signed-off-by: Guru Das Srinagesh Patches applied. Yours, Linus Walleij
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 6:44 PM David Hildenbrand wrote: > > drivers/gpu/drm/mcde/Kconfig > > drivers/gpu/drm/pl111/Kconfig > > drivers/gpu/drm/tve200/Kconfig > > I was assuming these are "real" dependencies. Will it also work without > DMA_CMA? It will mostly work but that is only because the reservations are mostly contiguous anyway because they are done early and are small. The hardware requires contiguous buffers in all three cases. I'm not sure I always got it right. > > certainly needs this as well, and pretty much anything that is > > selecting DRM_KMS_CMA_HELPER or > > DRM_GEM_CMA_HELPER "wants" DMA_CMA. > > "wants" as in "desires to use but can life without" or "wants" as in > "really needs it". ? I don't know the exact semantics of using DRM_KMS_CMA* without actually using DMA_CMA. I suspect small allocations will be contiguous and big allocations will start to fragment? but it's just my guess. I guess "really need it"? Yours, Linus Walleij
Re: [PATCH] pinctrl: ti: fix error return code of ti_iodelay_dt_node_to_map()
On Tue, Mar 30, 2021 at 10:39 AM angkery wrote: > From: Junlin Yang > > when devm_kcalloc fails, use -ENOMEM instead of -EINVAL, > and consistent with other devm_kcalloc return values. > > Signed-off-by: Junlin Yang Patch applied. Yours, Linus Walleij
Re: [PATCH v9 00/22] pinctrl: add BCM63XX pincontrol support
On Tue, Mar 30, 2021 at 10:57 AM Álvaro Fernández Rojas wrote: > > Now, what about a patch set for the IRQ support? :) > > If you could give me some guidance on that matter it would be much > appreciated, because your comments [1] are now outdated since I switched > to GPIO_REGMAP > [1] > http://patchwork.ozlabs.org/project/linux-gpio/patch/20210225164216.21124-3-nolt...@gmail.com/ I think it mostly holds: GPIOLIB_IRQCHIP should always be used if there is a reasonably straight-forward interrupts whether cascaded or hierarchical. Very few exceptions there. If there is one IRQ line per GPIO line, the hierarchical support should be used as outlined. GPIO_REGMAP should be mostly (famous last words) orthogonal. Yours, Linus Walleij
Re: [PATCH V2 0/3] Add GPIO support for PM7325, PM8350c, PMK8350 and PMR735A
On Thu, Apr 1, 2021 at 2:36 PM satya priya wrote: > satya priya (3): > pinctrl: qcom: spmi-gpio: Add support for four variants > dt-bindings: pinctrl: qcom-pmic-gpio: Update the binding to add four > new variants > dt-bindings: pinctrl: qcom-pmic-gpio: Convert qcom pmic gpio bindings > to YAML Please collect the ACKs and rebase like Björn says, sort stuff alphabetically and resend so I can try to apply it! The YAML conversion may need a nod from the DT people as well. Yours, Linus Walleij
Re: [PATCH 0/3] gpio-rockchip driver
On Mon, Mar 22, 2021 at 11:43 AM Jianqun Xu wrote: > Separate gpio driver from pinctrl driver. I tried to apply this too, but it fails, can you rebase on the pinctrl "devel" branch (I suppose the RK3568 driver got in the way). Yours, Linus Walleij
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 2:01 PM David Hildenbrand wrote: > > This is something you could do using a hidden helper symbol like > > > > config DRMA_ASPEED_GFX > > bool "Aspeed display driver" > > select DRM_WANT_CMA > > > > config DRM_WANT_CMA > > bool > > help > >Select this from any driver that benefits from CMA being enabled > > > > config DMA_CMA > > bool "Use CMA helpers for DRM" > > default DRM_WANT_CMA > > > > Arnd > > > > That's precisely what I had first, with an additional "WANT_CMA" -- but > looking at the number of such existing options (I was able to spot 1 !) If you do this it probably makes sense to fix a few other drivers Kconfig in the process. It's not just a problem with your driver. "my" drivers: drivers/gpu/drm/mcde/Kconfig drivers/gpu/drm/pl111/Kconfig drivers/gpu/drm/tve200/Kconfig certainly needs this as well, and pretty much anything that is selecting DRM_KMS_CMA_HELPER or DRM_GEM_CMA_HELPER "wants" DMA_CMA. Yours, Linus Walleij
Re: [PATCH v1] drivers: pinctrl: qcom: fix Kconfig dependency on GPIOLIB
On Mon, Mar 29, 2021 at 6:41 PM Julian Braha wrote: > > On Tuesday, March 2, 2021 9:46:04 AM EDT you wrote: > > On Thu, Feb 25, 2021 at 9:33 AM Julian Braha wrote: > > > > > When PINCTRL_MSM is enabled, and GPIOLIB is disabled, > > > Kbuild gives the following warning: > > > > > > WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP > > > Depends on [n]: GPIOLIB [=n] > > > Selected by [y]: > > > - PINCTRL_MSM [=y] && PINCTRL [=y] && (ARCH_QCOM || COMPILE_TEST [=y]) > > > > > > This is because PINCTRL_MSM selects GPIOLIB_IRQCHIP, > > > without selecting or depending on GPIOLIB, despite > > > GPIOLIB_IRQCHIP depending on GPIOLIB. Having PINCTRL_MSM > > > select GPIOLIB will cause a recursive dependency error. > > > > > > Signed-off-by: Julian Braha > > > > Does it work to just: > > > > select GPIOLIB > > > > instead? > > > > The driver needs the library so... > > > > Yours, > > Linus Walleij > > > > Hi Linus, > > Looks like I confused this patch with another one when > I responded last time. This config option cannot select > GPIOLIB, because it will cause a recursive dependency > error. > > Any other ideas? No we can apply the patch as-is but let Bjorn have a look at it first, I noticed he is not on the To: line of the original patch. You may need to resend with Bjorn Andersson in the recipients. Yours, Linus Walleij
Re: [PATCH v4] gpio: mpc8xxx: Add ACPI support
On Tue, Apr 6, 2021 at 3:49 AM Ran Wang wrote: > Could this version be accepted, or any comment/suggestion? Andy says yes, then it is a yes :) FWIW Acked-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH 32/32] pinctrl: update pin-control.rst references
On Thu, Apr 1, 2021 at 2:17 PM Mauro Carvalho Chehab
wrote:
> Changeset 5513b411ea5b ("Documentation: rename pinctl to pin-control")
> renamed: Documentation/driver-api/pinctl.rst
> to: Documentation/driver-api/pin-control.rst.
>
> Update the cross-references accordingly.
>
> Fixes: 5513b411ea5b ("Documentation: rename pinctl to pin-control")
> Signed-off-by: Mauro Carvalho Chehab
Reviewed-by: Linus Walleij
I assume you will apply this Mauro?
Yours,
Linus Walleij
Re: gemini: sl3516: Mainlining of NS 2502
};
};
But maybe your device is using GMAC1? I would try GMAC0 first anyway.
> The vendor source could be found at
> https://www.edimax.com/edimax/mw/cufiles/files/download/OpenSourceCode/transfer/Wireless/NAS/NAS-GPL-source.zip
Normally the kernel should be setting up device resources and
config under arch/arm/mach-sl2312 (never mind the ASIC number...)
but Storlink have hacked the kernel all over the place so the
relevant parts can be hard to find.
The interesting stuff relating to pin control appears when
you grep for SL2312_GLOBAL_BASE.
When the kernel boots some essentials are set up
already in init/main.c (hacky!). But it seems to be the same
as in all Storlink devices.
This part in drivers/net/sl2312_emac.c looks quite interesting:
#ifdef CONFIG_SL3516_ASIC
{
unsigned intval;
/* set GMAC global register */
val = readl(GMAC_GLOBAL_BASE_ADDR+0x10);
val = val | 0x005a;
writel(val,GMAC_GLOBAL_BASE_ADDR+0x10);
writel(0x07f007f0,GMAC_GLOBAL_BASE_ADDR+0x1c);
writel(0x,GMAC_GLOBAL_BASE_ADDR+0x20);
writel(0x,GMAC_GLOBAL_BASE_ADDR+0x24);
val = readl(GMAC_GLOBAL_BASE_ADDR+0x04);
if((val&(1<<20))==0){ // GMAC1 enable
val = readl(GMAC_GLOBAL_BASE_ADDR+0x30);
val = (val & 0xe7ff) | 0x0800;
writel(val,GMAC_GLOBAL_BASE_ADDR+0x30);
}
}
#endif
So we need to verify that this corresponds to what you set
up in your device tree (I do it by reading the data sheets and
coparing to the pinctrl-gemini.c code and defines...)
Then we have this:
/* define GPIO pin for MDC/MDIO */
// for gemini ASIC
#ifdef CONFIG_SL3516_ASIC
#define H_MDC_PIN 22
#define H_MDIO_PIN 21
#define G_MDC_PIN 22
#define G_MDIO_PIN 21
#else
(...)
This seems to correspond to your device tree so OK...
But it's annoying that we can't communicate with it.
This is usually because some other device is
"shading" the GPIO lines, i.e. hiding it.
drivers/net/sl_switch.c is not used on your platform (Vitesse switch)
and is a leftover from the Storlink reference design.
> > > BUT neither ethernet nor USB works.
> >
> > For USB try this patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git/commit/?h=gemini-usb=cbaf6cdf770b90de5f10bfa5112c679f1dffe948
> >
>
> One of the USB port is now working.
Hey nice! :D
The USB patch is not very widely tested, so it may need some shaping
up.
> Note that I have also started to work on the gemini crypto driver.
I saw some nice debug prints! :D
I always wanted to get that to work, nice that you're working on
it!
Yours,
Linus Walleij
Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error
On Wed, Apr 7, 2021 at 5:49 AM Dinghao Liu wrote: > When devm_request_threaded_irq() fails, we should decrease the > runtime PM counter to keep the counter balanced. But when > iio_device_register() fails, we need not to decrease it because > we have already decreased it before. > > Signed-off-by: Dinghao Liu Looks correct, this semantic ordering always confuse me a bit: Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: gemini: sl3516: Mainlining of NS 2502
On Mon, Apr 5, 2021 at 8:39 PM Corentin Labbe wrote:
> I own an Edimax NS 2502, which is a NAS based on StormLinix 3516.
> I successfully upgraded it with a recent Linux.
Pretty cool!
> mdio0: ethernet-phy {
> compatible = "virtual,mdio-gpio";
> gpios = < 22 GPIO_ACTIVE_HIGH>, /* MDC */
> < 21 GPIO_ACTIVE_HIGH>; /* MDIO */
> #address-cells = <1>;
> #size-cells = <0>;
> phy0: ethernet-phy@1 {
> reg = <1>;
> device_type = "ethernet-phy";
> };
> };
This looks like the most typical way to attach an MDIO phy.
I always try to identify the exact component used on the board. Do you have
a high res board photo?
Realtek RTL82111 is the most common configuration.
Compare to the D-Linux DNS-313 DTS:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/gemini-dlink-dns-313.dts
Try just copying the whole pinctrl-gmii section!
> syscon: syscon@4000 {
> pinctrl {
> /*
> * gpio0agrp cover line 0-4
> * gpio0bgrp cover line 5
> */
> gpio0_default_pins: pinctrl-gpio0 {
> mux {
> function = "gpio0";
> groups = "gpio0agrp",
> "gpio0bgrp";
> };
> };
Change groups to
groups = "gpio0agrp", "gpio0bgrp", "gpio0hgrp";
So you mux in group h which is where the GPIO 21, 22 go out
to the MDIO on 3516 IIUC. The right mux out is pretty important,
if you have vendor source code, please share so I can check how
they set it up.
> BUT neither ethernet nor USB works.
For USB try this patch:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git/commit/?h=gemini-usb=cbaf6cdf770b90de5f10bfa5112c679f1dffe948
Pls report progress! I hope we can mainline this device.
Yours,
Linus Walleij
Re: [PATCH RFC/RFT 1/1] misc: add simple logic analyzer using polling
On Tue, Mar 30, 2021 at 10:58 AM Wolfram Sang
wrote:
> This is a simple logic analyzer using GPIO polling. It comes with a
> script to isolate a CPU for polling. While this is definately not a
> production level analyzer, it can be a helpful first view when remote
> debugging. Read the documentation for details.
>
> Signed-off-by: Wolfram Sang
I am a great supporter of this idea.
When we created gpiod_get_array_value() and friends, the idea
was exactly to be able to do things like this. It's a good way to
utilize the fact that several GPIO lines can often be read from a single
register read.
> +i2c-analyzer {
> +compatible = "gpio-logic-analyzer";
> +probe-gpios = < 21 GPIO_OPEN_DRAIN>, < 4
> GPIO_OPEN_DRAIN>;
> +probe-names = "SCL", "SDA";
> +};
> +
> +The binding documentation is in the ``misc`` folder of the Kernel binding
> +documentation.
(...)
> +++ b/Documentation/devicetree/bindings/misc/gpio-logic-analyzer.yaml
When other debugging tools for GPIO got DT bindings it was concluded that
it is possible to create bindings like this for debugging without even
specifying
any formal bindings. They are just for debugging after all.
Personally I like the bindings anyway.
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
I would consider housing this tool under drivers/gpio actually.
We have other funky things like gpio-sim and gpio-aggregator
so why not.
I would create a Kconfig menu with "GPIO hardware hacking tools".
But Bartosz would need to agree on that idea.
> +config GPIO_LOGIC_ANALYZER
> + tristate "Simple GPIO logic analyzer"
> + depends on GPIOLIB || COMPILE_TEST
> + help
depends on EXPERT
I would say. Definitely not something for the average user.
Yours,
Linus Walleij
Re: [PATCH v2] pinctrl: bcm: bcm6362: fix warning
On Tue, Mar 30, 2021 at 12:32 PM Álvaro Fernández Rojas
wrote:
> The current implementation of bcm6362_set_gpio() produces the following
> warning on x86_64:
> drivers/pinctrl/bcm/pinctrl-bcm6362.c: In function 'bcm6362_set_gpio':
> drivers/pinctrl/bcm/pinctrl-bcm6362.c:503:8: warning: cast from pointer to
> integer of different size [-Wpointer-to-int-cast]
> 503 |(uint32_t) desc->drv_data, 0);
> |^
>
> Modify the code to make it similar to bcm63268_set_gpio() in order to fix
> the warning.
>
> Fixes: 705791e23ecd ("pinctrl: add a pincontrol driver for BCM6362")
> Signed-off-by: Álvaro Fernández Rojas
Patch applied! Thanks!
Yours,
Linus Walleij
[GIT PULL] pin control fixes for the v5.12 kernel
Hi Linus, here are some overly ripe fixes for the v5.12 kernel. I should have sent earlier but had my head stuck in GDB. All are driver fixes. Details in the signed tag. Please pull it in! Yours, Linus Walleij The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15: Linux 5.12-rc2 (2021-03-05 17:33:41 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git tags/pinctrl-v5.12-2 for you to fetch changes up to ba845907b23a6584e5944f6fbffda3efb010c28b: Merge tag 'intel-pinctrl-v5.12-3' of gitolite.kernel.org:pub/scm/linux/kernel/git/pinctrl/intel into fixes (2021-03-30 00:46:49 +0200) Pin control fixes for the v5.12 kernel cycle: - Fix up some Intel GPIO base calculations. - Fix a register offset in the Microchip driver. - Fix suspend/resume bug in the Rockchip driver. - Default pull up strength in the Qualcomm LPASS driver. - Fix two pingroup offsets in the Qualcomm SC7280 driver. - Fix SDC1 register offset in the Qualcomm SC7280 driver. - Fix a nasty string concatenation in the Qualcomm SDX55 driver. - Check the REVID register to see if the device is real or virtualized during virtualization in the Intel driver. Andy Shevchenko (1): pinctrl: intel: Show the GPIO base calculation explicitly Arnd Bergmann (1): pinctrl: qcom: fix unintentional string concatenation Jonathan Marek (1): pinctrl: qcom: lpass lpi: use default pullup/strength values Lars Povlsen (1): pinctrl: microchip-sgpio: Fix wrong register offset for IRQ trigger Linus Walleij (2): Merge tag 'intel-pinctrl-v5.12-2' of gitolite.kernel.org:pub/scm/linux/kernel/git/pinctrl/intel into fixes Merge tag 'intel-pinctrl-v5.12-3' of gitolite.kernel.org:pub/scm/linux/kernel/git/pinctrl/intel into fixes Rajendra Nayak (2): pinctrl: qcom: sc7280: Fix SDC_QDSD_PINGROUP and UFS_RESET offsets pinctrl: qcom: sc7280: Fix SDC1_RCLK configurations Roger Pau Monne (1): pinctrl: intel: check REVID register value for device presence Wang Panzhenzhuan (1): pinctrl: rockchip: fix restore error in resume drivers/pinctrl/intel/pinctrl-intel.c | 9 - drivers/pinctrl/pinctrl-microchip-sgpio.c | 2 +- drivers/pinctrl/pinctrl-rockchip.c| 13 - drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 2 +- drivers/pinctrl/qcom/pinctrl-sc7280.c | 16 drivers/pinctrl/qcom/pinctrl-sdx55.c | 2 +- 6 files changed, 27 insertions(+), 17 deletions(-)
Re: [PATCH] pinctrl: microchip: fix array overflow
On Sun, Mar 28, 2021 at 7:18 PM Lars Povlsen wrote:
> Linus Walleij writes:
>
> > On Tue, Mar 23, 2021 at 2:10 PM Arnd Bergmann wrote:
> >
> >> From: Arnd Bergmann
> >>
> >> Building with 'make W=1' shows an array overflow:
> >>
> >> drivers/pinctrl/pinctrl-microchip-sgpio.c: In function
> >> 'microchip_sgpio_irq_settype':
> >> drivers/pinctrl/pinctrl-microchip-sgpio.c:154:39: error: array subscript
> >> 10 is above array bounds of 'const u8[10]' {aka 'const unsigned char[10]'}
> >> [-Werror=array-bounds]
> >> 154 | u32 regoff = priv->properties->regoff[rno] + off;
> >> | ^
> >> drivers/pinctrl/pinctrl-microchip-sgpio.c:55:5: note: while referencing
> >> 'regoff'
> >>55 | u8 regoff[MAXREG];
> >> | ^~
> >>
> >> It's not clear to me what was meant here, my best guess is that the
> >> offset should have been applied to the third argument instead of the
> >> second.
> >>
> >> Fixes: be2dc859abd4 ("pinctrl: pinctrl-microchip-sgpio: Add irq support
> >> (for sparx5)")
> >> Signed-off-by: Arnd Bergmann
> >
> > Patch applied.
> >
> > Yours,
> > Linus Walleij
>
> I don't understand - I submitted a fix for this already in February
> (reported by Gustavo). It took some time for you to get it ack'ed - but
> you did (Feb 1st).
>
> Did it end up getting dropped?
No I ended up with your fix in fixes, then forgot about it and applied
Arnds fix to devel (for-next) and ended up getting a conflict in my
face.
Last night I rebased devel, dropped Arnds patch and thus solved
the conflict.
Yours,
Linus Walleij
Re: linux-next: build warning after merge of the pinctrl tree
On Tue, Mar 30, 2021 at 8:07 AM Stephen Rothwell wrote:
> After merging the pinctrl tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
>
> drivers/pinctrl/bcm/pinctrl-bcm6362.c: In function 'bcm6362_set_gpio':
> drivers/pinctrl/bcm/pinctrl-bcm6362.c:503:8: warning: cast from pointer to
> integer of different size [-Wpointer-to-int-cast]
> 503 |(uint32_t) desc->drv_data, 0);
> |^
>
> Introduced by commit
>
> 705791e23ecd ("pinctrl: add a pincontrol driver for BCM6362")
Ha! The 0day robot didn't see it. Good that we have so many layers.
I bet Álvaro will have a patch to fix it in no time.
Yours,
Linus Walleij
Re: [PATCH v2 01/13] gpio: Add Elba SoC gpio driver for spi cs control
On Mon, Mar 29, 2021 at 3:59 AM Brad Larson wrote: > This GPIO driver is for the Pensando Elba SoC which > provides control of four chip selects on two SPI busses. > > Signed-off-by: Brad Larson You have not addressed mine nor Andy's comments on v1. Go back, read and reply, and rewrite. Yours, Linus Walleij
Re: [PATCH v2 13/13] gpio: Use linux/gpio/driver.h
On Mon, Mar 29, 2021 at 4:00 AM Brad Larson wrote: > New drivers should include instead > of legacy . > > Signed-off-by: Brad Larson Fold into patch 1 as indicated by Greg. Yours, Linus Walleij
Re: [PATCH v9 01/22] gpio: guard gpiochip_irqchip_add_domain() with GPIOLIB_IRQCHIP
On Fri, Mar 26, 2021 at 2:14 PM Bartosz Golaszewski wrote: > I suppose this will go through the pinctrl tree. Yups I applied them. To make things mess-resistant I have applied them on an immutable branch and then merged that to devel: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=ib-bcm63xx If we get problems between GPIO and pin control you can pull this in, but let's hope not! Yours, Linus Walleij
Re: [PATCH v9 00/22] pinctrl: add BCM63XX pincontrol support
On Wed, Mar 24, 2021 at 9:19 AM Álvaro Fernández Rojas wrote: > This patchset adds appropriate binding documentation and drivers for > pin controller cores found in the BCM63XX MIPS SoCs currently supported. I have applied and pushed the v9 patch series with all the ACKs to the "devel" branch so the build servers can churn at it! Later today I will integrate it into linux-next. Any remaining issues can certainly be fixed in-tree. Thanks for your perseverance in cleaning up these SoCs!! Now, what about a patch set for the IRQ support? :) Yours, Linus Walleij
Re: [PATCH v2 00/13] Driver of Intel(R) Gaussian & Neural Accelerator
On Wed, Mar 24, 2021 at 7:39 PM Maciej Kwapulinski wrote: > This submission is a kernel driver to support Intel(R) Gaussian & Neural > Accelerator (Intel(R) GNA). Intel(R) GNA is a PCI-based neural co-processor > available on multiple Intel platforms. I clearly remember Olof Johansson talking about the potential need of creating a kernel subsystem for inference engines, so I believe he wants to be in on this discussion. There is already misc/habanalabs, and I personally feel this is already going in the same direction as did pin control before we standardized it (somewhat), with vendors claiming they are all necessarily different. So are they necessarily different? New frontiers in the Wild West every vendor shooting from the hip without any attempts at standardizing this thing? Habanalabs was first at this and they made it in, has there been any attempt to see if the two drivers could actually share code or have anything in common? Could they share interfaces to userspace? That kind of thing. In the end what kernel users want is to be able to write a userspace making use of any kind of inference/statistics engine without having to worry about the underlying hardware, this is what abstractions are for. > The driver works with Intel(R) libraries in user space. The Intel(R) driver > exposes a few IOCTL interfaces for use by libraries in user space. The > libraries are open sourced and are available at: > https://github.com/intel/gna This is nice. Have you made any attempts to cooperate with anyone else in the world on this, or is this Intel's personal playground? Yours, Linus Walleij
Re: [PATCH] tools: gpio-utils: fix various kernel-doc warnings
On Thu, Mar 25, 2021 at 6:56 PM Randy Dunlap wrote: > Fix several problems in kernel-doc notation in gpio-utils.c. > > gpio-utils.c:37: warning: Incorrect use of kernel-doc format: * > gpiotools_request_line() - request gpio lines in a gpiochip > gpio-utils.c:61: warning: expecting prototype for doc(). Prototype was for > gpiotools_request_line() instead > gpio-utils.c:265: warning: Excess function parameter 'value' description in > 'gpiotools_sets' > gpio-utils.c:1: warning: 'gpiotools_request_lines' not found > > Signed-off-by: Randy Dunlap > Cc: Bartosz Golaszewski > Cc: Linus Walleij > Cc: linux-g...@vger.kernel.org Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH] pinctrl: qcom: fix unintentional string concatenation
On Tue, Mar 23, 2021 at 2:17 PM Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> clang is clearly correct to point out a typo in a silly
> array of strings:
>
> drivers/pinctrl/qcom/pinctrl-sdx55.c:426:61: error: suspicious concatenation
> of string literals in an array initialization; did you mean to separate the
> elements with a comma? [-Werror,-Wstring-concatenation]
> "gpio14", "gpio15", "gpio16", "gpio17", "gpio18", "gpio19" "gpio20",
> "gpio21", "gpio22",
>^
> Add the missing comma that must have accidentally been removed.
>
> Fixes: ac43c44a7a37 ("pinctrl: qcom: Add SDX55 pincontrol driver")
> Signed-off-by: Arnd Bergmann
Patch applied.
Yours,
Linus Walleij
Re: [PATCH v2] ARM: Implement Clang's SLS mitigation
Hi Will, I went back and found this feedback which is kind of the heart of the issues regarding SLS. On Wed, Feb 17, 2021 at 10:51 AM Will Deacon wrote: > The big problem I have with this is that it's a compile-time decision. > For the other spectre crap we have a combination of the "mitigations=off" > command-line and CPU detection to avoid the cost of the mitigation where > it is not deemed necessary. For newcomers, the way this works today can be found in e.g.: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/proton-pack.c mitigations=off turns off Spectre v2 and v4 mitigations. AFAICT this is achived with misc parameterization to firmware and hypervisors and no runtime-patching of any code at all? (On ARM32 it has no effect whatsoever, we just turn on all spectre v2 mitigations by default. No runtime choice.) The way I understand it is that for SLS the compiler must at least put in some kind of placeholders, but that it *might* be possible to do runtime mitigations on top of that. We need feedback from the compiler people as to what is possible here. If it is *not* possible to mitigate at run-time, then I don't know what is the right thing to do. Certainly not to turn it on by default as is done today? > So I think that either we enable this unconditionally, or we don't enable it > at all (and people can hack their CFLAGS themselves if they want to). It > would be helpful for one of the Arm folks to chime in, as I'm yet to see any > evidence that this is actually exploitable. (...) > Is it any worse that Spectre-v1, > where we _don't_ have a compiler mitigation? There is such a compiler mitigation for Spectre v1, under the name "Speculative load hardening" the kernel is not (yet) enabling it. https://llvm.org/docs/SpeculativeLoadHardening.html it comes with the intuitive command line switch -mspeculative-load-hardening Certainly a separate patch can add speculative load hardening support on top of this, or before this patch, if there is desire and/or feels like a more coherent approach. As the article says "The performance overhead of this style of comprehensive mitigation is very high (...) most large applications seeing a 30% overhead or less." I suppose it can be enabled while compiling the kernel just like this patch enables -mharden-sls=all I don't know if your comment means that if we enable one of them we should just as well enable both or none as otherwise there is no real protection, as attackers can just use the other similar attack vector? > Finally, do we have to worry about our assembly code? AFAICT yes, and you seem to have hardened Aarch64's ERET:s which seemed especially vulnerable in commit 679db70801da9fda91d26caf13bf5b5ccc74e8e8 "arm64: entry: Place an SB sequence following an ERET instruction" Link for people without kernel source: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=679db70801da9fda91d26caf13bf5b5ccc74e8e8 So it seems the most vulnerable spot was already fixed by you, thanks! But I bet there are some more spots. Yours, Linus Walleij
Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
On Wed, Mar 24, 2021 at 8:29 AM Matti Vaittinen wrote: > Support GPO(s) found from ROHM BD71815 power management IC. The IC has two > GPO pins but only one is properly documented in data-sheet. The driver > exposes by default only the documented GPO. The second GPO is connected to > E5 pin and is marked as GND in data-sheet. Control for this undocumented > pin can be enabled using a special DT property. > > This driver is derived from work by Peter Yang > although not so much of original is left. > > Signed-off-by: Matti Vaittinen > --- > Changes since v3: > - No changes This looks OK to me: Acked-by: Linus Walleij It could potentially (like the other Rohm GPIO MFD PMIC drivers) make some use of the gpio regmap library, but we have some pending changes for that so look into it after the next merge window. I.e. for your TODO: look at the GPIO_REGMAP helper. Yours, Linus Walleij
Re: [PATCH] pinctrl: microchip: fix array overflow
On Tue, Mar 23, 2021 at 2:10 PM Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> Building with 'make W=1' shows an array overflow:
>
> drivers/pinctrl/pinctrl-microchip-sgpio.c: In function
> 'microchip_sgpio_irq_settype':
> drivers/pinctrl/pinctrl-microchip-sgpio.c:154:39: error: array subscript 10
> is above array bounds of 'const u8[10]' {aka 'const unsigned char[10]'}
> [-Werror=array-bounds]
> 154 | u32 regoff = priv->properties->regoff[rno] + off;
> | ^
> drivers/pinctrl/pinctrl-microchip-sgpio.c:55:5: note: while referencing
> 'regoff'
>55 | u8 regoff[MAXREG];
> | ^~
>
> It's not clear to me what was meant here, my best guess is that the
> offset should have been applied to the third argument instead of the
> second.
>
> Fixes: be2dc859abd4 ("pinctrl: pinctrl-microchip-sgpio: Add irq support (for
> sparx5)")
> Signed-off-by: Arnd Bergmann
Patch applied.
Yours,
Linus Walleij
Re: [PATCH v5 00/11] gpio: implement the configfs testing module
On Mon, Mar 22, 2021 at 3:32 PM Bartosz Golaszewski wrote: > FYI The configfs patches from this series have been on the mailing > list for months (long before the GPIO part) and have been re-sent > several times. You have neither acked or opposed these changes. I > don't want to delay the new testing driver anymore so I intend to > apply the entire series and take it upstream through the GPIO tree by > the end of this week. I say go ahead. Acked-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH] Documentation: gpio: chip should be plural
On Tue, Mar 23, 2021 at 3:56 PM Bryan Brattlof wrote: > Signed-off-by: Bryan Brattlof Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH] drivers: pinctrl: Remove duplicate include of io.h
On Tue, Mar 23, 2021 at 2:37 AM Wan Jiabing wrote: > linux/io.h has been included at line 6, so remove the > duplicate include at line 18. > > Signed-off-by: Wan Jiabing Patch applied! Yours, Linus Walleij
Re: [PATCH v2 6/7] usb: gadget: pch_udc: Initialize device pointer before use
On Tue, Mar 23, 2021 at 4:36 PM Andy Shevchenko
wrote:
> During conversion to use GPIO descriptors the device pointer,
> which is applied to devm_gpiod_get(), is not yet initialized.
>
> Move initialization in the ->probe() in order to have it set before use.
>
> Fixes: e20849a8c883 ("usb: gadget: pch_udc: Convert to use GPIO descriptors")
> Signed-off-by: Andy Shevchenko
Ooops sorry.
Reviewed-by: Linus Walleij
Yours,
Linus Walleij
Re: [PATCH v2 1/7] usb: gadget: pch_udc: Replace cpu_to_le32() by lower_32_bits()
On Tue, Mar 23, 2021 at 4:36 PM Andy Shevchenko
wrote:
> Either way ~0 will be in the correct byte order, hence
> replace cpu_to_le32() by lower_32_bits(). Moreover,
> it makes sparse happy, otherwise it complains:
>
> .../pch_udc.c:1813:27: warning: incorrect type in assignment (different base
> types)
> .../pch_udc.c:1813:27:expected unsigned int [usertype] dataptr
> .../pch_udc.c:1813:27:got restricted __le32 [usertype]
>
> Fixes: f646cf94520e ("USB device driver of Topcliff PCH")
> Signed-off-by: Andy Shevchenko
Nice fix! Also easier to understand.
Reviewed-by: Linus Walleij
Yours,
Linus Walleij
Re: [PATCH] pinctrl: PINCTRL_ROCKCHIP should depend on ARCH_ROCKCHIP
On Tue, Mar 16, 2021 at 2:41 PM Geert Uytterhoeven
wrote:
> The Rockchip GPIO and pin control modules are only present on Rockchip
> SoCs. Hence add a dependency on ARCH_ROCKCHIP, to prevent asking the
> user about this driver when configuring a kernel without Rockchip
> platform support.
>
> Note that before, the PINCTRL_ROCKCHIP symbol was not visible, and
> automatically selected when needed. By making it tristate and
> user-selectable, it became visible for everyone.
>
> Fixes: be786ac5a6c4bf4e ("pinctrl: rockchip: make driver be tristate module")
> Signed-off-by: Geert Uytterhoeven
Patch applied.
I saw this was causing issues on S390.
Yours,
Linus Walleij
Re: [PATCH] pinctrl: add lock in mtk_rmw function.
On Sun, Mar 21, 2021 at 4:32 AM Zhiyong Tao wrote: > When multiple threads operate on the same register resource > which include multiple pin, It will make the register resource > wrong to control. So we add lock to avoid the case. > > Signed-off-by: Zhiyong Tao Patch applied! Yours, Linus Walleij
Re: [RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
On Thu, Mar 18, 2021 at 9:00 AM Enrico Weigelt, metux IT consult wrote: > Is there some compact notation for swnode's that's as small and simple > as some piece of DTS ? Yes it's really neat. It's all in and examples in e.g. the testsuite: drivers/base/test/property-entry-test.c You can just grep for PROPERTY_ENTRY and you find some examples of how we use it. Yours, Linus Walleij
Re: [PATCH v4 2/3] dt-bindings: pinctrl: Add binding for ZynqMP pinctrl driver
On Wed, Mar 17, 2021 at 9:26 AM Sai Krishna Potthuri wrote: > + io-standard: > +description: > + Selects the IO standard for MIO pins, this is driver specific. > +$ref: "/schemas/types.yaml#/definitions/uint32" > +enum: [0, 1] As concluded from driver review, replace this with power-source which is already defined in Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml Yours, Linus Walleij
Re: [PATCH v4 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support
On Mon, Mar 22, 2021 at 4:25 PM Sai Krishna Potthuri wrote: > [Andy] > > > > > + PIN_CONFIG_IOSTANDARD = PIN_CONFIG_END + 1, }; > > > > > > > > I'm lost here. What is IO standard exactly? Why it can't be in > > > > generic headers? > > > It represents LVCMOS 3.3 volts/ LVCMOS 1.8 volts. > > > Since this is specific to Xilinx ZynqMP platform, considered to be > > > added in the driver file. > > > > So, why can't we create a couple of bits to represent this voltages in the > > generic header and gain usability for others as well? > > I see some drivers are maintaining the configuration list in the driver file, > if > the configuration is specific to the driver. > > We can move this to generic header if it is used by others as well. > Ok, will wait for Linus to comment. > > > > Linus? While it is fine to add custom pin config options to pin controllers for hopelessly idiomatic stuff, this does look like it should be using PIN_CONFIG_POWER_SOURCE with the voltage rail as parameter, see include/linux/pinctrl/pinconf-generic.h If you're not using that then tell us why. Yours, Linus Walleij
Re: [PATCH v2 7/7] usb: gadget: pch_udc: Provide a GPIO line used on Intel Minnowboard (v1)
On Tue, Mar 23, 2021 at 4:36 PM Andy Shevchenko
wrote:
> Intel Minnowboard (v1) uses SCH GPIO line SUS7 (i.e. 12)
> for VBUS sense. Provide a DMI based quirk to have it's being used.
>
> Fixes: e20849a8c883 ("usb: gadget: pch_udc: Convert to use GPIO descriptors")
> Signed-off-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Yours,
Linus Walleij
Re: [PATCH v1 6/6] usb: gadget: pch_udc: Provide a GPIO line used on Intel Minnowboard (v1)
On Mon, Mar 22, 2021 at 10:11 PM Andy Shevchenko
wrote:
> Intel Minnowboard (v1) uses SCH GPIO line SUS7 (i.e. 12)
> for VBUS sense. Provide a DMI based quirk to have it's being used.
>
> Fixes: e20849a8c883 ("usb: gadget: pch_udc: Convert to use GPIO descriptors")
> Signed-off-by: Andy Shevchenko
Excellent solution!
Reviewed-by: Linus Walleij
Yours,
Linus Walleij
Re: [PATCH v5 2/2] gpio: sch: Hook into ACPI GPE handler to catch GPIO edge events
On Wed, Mar 17, 2021 at 4:19 PM Andy Shevchenko wrote: > Neither the ACPI description on Intel Minnowboard (v1) platform provides > the required information to establish a generic handling nor the hardware > capable of doing it. According to the data sheet the hardware can generate > SCI events. Therefore, we need to hook from the driver into GPE handler of > the ACPI subsystem in order to catch and report GPIO-related events. > > Validated on the Inlel Minnowboard (v1) platform. > > Signed-off-by: Andy Shevchenko Looks good to me: Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v5 1/2] gpio: sch: Add edge event support
On Wed, Mar 17, 2021 at 4:19 PM Andy Shevchenko wrote: > From: Jan Kiszka > > Add the required infrastructure to enable and report edge events > of the pins to the GPIO core. The actual hook-up of the event interrupt > will happen separately. > > Signed-off-by: Jan Kiszka > Co-developed-by: Andy Shevchenko > Signed-off-by: Andy Shevchenko I can't believe it that nobody added irq support to this driver for 10 years given how widely deployed it is! (Good work.) Don't you need to add select GPIOLIB_IRQCHIP to Kconfig? So the gpio_chip contains the .irq member you're using. > + sch->irqchip.name = "sch_gpio"; > + sch->irqchip.irq_ack = sch_irq_ack; > + sch->irqchip.irq_mask = sch_irq_mask; > + sch->irqchip.irq_unmask = sch_irq_unmask; > + sch->irqchip.irq_set_type = sch_irq_type; > + > + sch->chip.irq.chip = >irqchip; > + sch->chip.irq.num_parents = 0; > + sch->chip.irq.parents = NULL; > + sch->chip.irq.parent_handler = NULL; > + sch->chip.irq.default_type = IRQ_TYPE_NONE; > + sch->chip.irq.handler = handle_bad_irq; I always add a local variable like: struct gpio_irq_chip *girq; And assign with the arrow, so as to make it easier to read: girq->parent_handler = NULL etc. +/- the above: Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v4 0/3] Fix pinctrl-single pcs_pin_dbg_show()
On Fri, Mar 19, 2021 at 4:22 PM Hanna Hawa wrote: > These patches fix the pcs_pin_dbg_show() function for the scenario where > a single register controls multiple pins (i.e. bits_per_mux is not zero) > Additionally, the common formula is moved to a separate function to > allow reuse. This v4 patch set applied! Yours, Linus Walleij
Re: [PATCH] linux/gpio/driver.h: some edits for clarity
On Tue, Mar 23, 2021 at 11:19 PM Randy Dunlap wrote: > Fix a few typos and some punctuation. > Also, change CONFIG_OF to CONFIG_OF_GPIO in one comment. > > Signed-off-by: Randy Dunlap > Cc: Linus Walleij > Cc: Bartosz Golaszewski > Cc: linux-g...@vger.kernel.org Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [RESEND 1/1] arch: arm: mach-at91: pm: Move prototypes to mutually included header
On Wed, Mar 3, 2021 at 4:50 PM Lee Jones wrote: > On Wed, 03 Mar 2021, Alexandre Belloni wrote: > > On 03/03/2021 12:41:49+, Lee Jones wrote: > > > Both the caller and the supplier's source file should have access to > > > the include file containing the prototypes. > > > > > > Fixes the following W=1 kernel build warning(s): > > > > > > drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype > > > for ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes] > > > 1637 | void at91_pinctrl_gpio_suspend(void) > > > | ^ > > > drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype > > > for ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes] > > > 1661 | void at91_pinctrl_gpio_resume(void) > > > | ^~~~ > > > > > > Cc: Russell King > > > Cc: Nicolas Ferre > > > Cc: Alexandre Belloni > > > Cc: Ludovic Desroches > > > Signed-off-by: Lee Jones > > > > I'm pretty sure you had my ack on v3 ;) > > > > Acked-by: Alexandre Belloni > > > > or again, alternatively, I can apply it with Linus' ack > > That would be my preference, thanks. Acked-by: Linus Walleij Yours, Linus Walleij
Re: [GIT PULL] Immutable branch between MFD and Power due for the v5.13 merge window
On Tue, Mar 23, 2021 at 9:57 AM Lee Jones wrote: > git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-power-v5.13 Thanks so much for fixing this Lee! Sebastian: if you decide to pull this in I can iterate some more patches to the AB8500 BM code this merge window, else I will just defer to after v5.13-rc1. Yours, Linus Walleij
Re: GTE - The hardware timestamping engine
On Mon, Mar 22, 2021 at 9:17 PM Dipen Patel wrote: > My follow-up concerns on both Linus's and Kent's feedback: > > 1. Please correct me if I am wrong, lineevent in the gpiolib* is only > serves the userspace clients. > 1.a What about kernel drivers wanting to use this feature for monitoring its > GPIO lines, see gyroscope example somewhere below. In that regards, > lineevent implementation is not sufficient. > 1.b Are you also implying to extend lineevent implementation to kernel > drivers? I was talking about lineevent because you mentioned things like motors and robotics, and those things are traditionally not run in kernelspace because they are not generic hardware that fit in the kernel subsystems. Normally industrial automatic control tasks are run in a userspace thread with some realtime priority. As Kent says, in-kernel events are exclusively using IRQ as mechanism, and should be modeled as IRQs. Then the question is how you join the timestamp with the IRQ. GPIO chips are just some kind of irqchip in this regard, we reuse the irqchip infrastructure in the kernel for all GPIO drivers that generate "events" in response to state transitions on digital lines. > >> And certainly you will also want to use this timestamp for > >> IIO devices? If it is just GPIOs and IRQs today, it will be > >> gyroscopes and accelerometers tomorrow, am I right? > >> > > Gyroscope, accelerometers or any IIO are built on top of i2c/spi and/or GPIOs. > So they are covered as long as they serve as client to GTE framework, For > example, if gyroscope uses GPIO as an interrupt to indicate frame > ready, GTE could timestamp that GPIO as well any IRQs like i2c transaction > complete IRQ. To this to happen, gycroscope then register itself with > GTE framework and enable required signals that it interfaces/interested with. I think there are IIO devices that provide their own hardware timestamp and as such they might want to use that, so the mechanism need to be generic enough that a certain hardware timestamp can be selected sooner or later. But let's not overcomplicate things for now. Yours, Linus Walleij
Re: include/linux/unaligned/be_byteshift.h:46:19: error: redefinition of 'get_unaligned_be32'
On Fri, Mar 19, 2021 at 10:57 AM Andy Shevchenko wrote: > On Fri, Mar 19, 2021 at 9:05 AM kernel test robot wrote: > > > > Hi Linus, > > > > FYI, the error/warning still remains. (...) > >In file included from include/linux/build_bug.h:5, > > from include/linux/bitfield.h:10, > > from drivers/iio/magnetometer/yamaha-yas530.c:22: > > Isn't it fixed already somewhere? It is, I think Jonathan already applied the fix, it is just waiting to percolate up to Greg and then to Torvals: https://lore.kernel.org/linux-iio/20210221154511.75b3d8a6@archlinux/ >> Kconfig warnings: (for reference only) >>WARNING: unmet direct dependencies detected for SND_ATMEL_SOC_PDC >>Depends on SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC && HAS_DMA >>Selected by >>- SND_ATMEL_SOC_SSC && SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC >>- SND_ATMEL_SOC_SSC_PDC && SOUND && !UML && SND && SND_SOC && >> SND_ATMEL_SOC && ATMEL_SSC > > This one though is interesting. This looks like nothing to me :/ As confused as ever about Kconfig. Yours, Linus Walleij
Re: [PATCH 0/3] gpio-rockchip driver
Hi Jianqun, On Mon, Mar 22, 2021 at 11:43 AM Jianqun Xu wrote: > Separate gpio driver from pinctrl driver. > > Jianqun Xu (3): > pinctrl/rockchip: separate struct rockchip_pin_bank to a head file > pinctrl/pinctrl-rockchip.h: add pinctrl device to gpio bank struct > gpio: separate gpio driver from pinctrl-rockchip driver > > drivers/gpio/Kconfig | 8 + > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-rockchip.c | 650 + > drivers/pinctrl/pinctrl-rockchip.c | 909 + > drivers/pinctrl/pinctrl-rockchip.h | 246 > 5 files changed, 924 insertions(+), 890 deletions(-) This is a very nice change (separation of concerns!) I'm just pinging to include Bartosz on this patch set so he knows a new GPIO driver may be appearing through the pinctrl tree (and he may want to take a look). Yours, Linus Walleij
Re: [PATCH v1 1/1] gpio: Support interrupts in gpio-mlxbf2.c
On Wed, Mar 10, 2021 at 9:38 PM Asmaa Mnebhi wrote: > > That's fine, the hardware description model (I guess in your case > > ACPI) should take care of that. > > > We cannot really pass it through the ACPI table because the ACPI > table is common to all BlueField-2 boards. And each board may have > a different GPIO pin associated with a particular function. This is > why we use ACPI properties instead of GpioInt(). So that the > bootloader can change the GPIO pin value based on the board > id detected at boot time. (...) > Yes. It would belong in the ACPI table if we had a different ACPI > table for each board. But unfortunately that is not the case. You have to agree with Andy about all ACPI details. Andy is the ACPI GPIO maintainer and we cannot merge a patch with any kind of ACPI support without his ACK, so hash it out as he wants it. The only people on the planet that can make me think otherwise is if Rafael Wysocki and Mika Westerberg say something else, which is *extremely* unlikely. If you need to do workarounds because of broken ACPI tables, you need to convince the ACPI maintainers that there is no way you can fix these tables so it needs to be fixed in the kernel. It is not something for the GPIO maintainers to decide. To continue that argument please mail these people in the MAINTAINERS file, Andy and Mika Westerberg and have a discussion with the kernel ACPI community: ACPI M: "Rafael J. Wysocki" M: Len Brown L: linux-a...@vger.kernel.org Yours, Linus Walleij
Re: [PATCH v5] ARM: Implement SLS mitigation
On Wed, Mar 10, 2021 at 5:43 AM Jian Cai wrote: > On Sat, Mar 6, 2021 at 4:25 AM Linus Walleij wrote: > > On Fri, Mar 5, 2021 at 12:23 AM Jian Cai wrote: > > > On Wed, Mar 3, 2021 at 7:04 AM Linus Walleij > > > wrote: > > > I think gcc also has these options. > > > https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html > > > > And how does that work with this part of your patch: > > > > +#define SLS_TEXT \ > > + ALIGN_FUNCTION(); \ > > + *(.text.__llvm_slsblr_thunk_*) > > > > This does not look compiler agnostic? > > You are right, GCC does generate different oraphan section names. I > will address it in the next version of the patch. Also it seems only > arm64 gcc supports -mharden-sls=* at this moment, arm32 gcc does not > support it yet. I don't know if there is any plan to implement it for > 32-bit gcc, but should we patch arm32 linker script preemptively, > assuming the sections will be named with the same pattern like how > clang does so the kernel would not fail to boot when the flag is > implemented? I think the best thing is to have something like this: Implement a macro such as this in include/linux/compiler-clang.h #define SLS_TEXT_SECTION *(.text.__llvm_slsblr_thunk_*) then the corresponding in include/linux/compiler-gcc.h but here also add a #define SLS_TEXT_SECTION #error "no compiler support" if the compiler version does not have this. I don't know the exact best approach sadly, as the patch looks now it seems a bit fragile, I wonder if you get linker warnings when this section is unused? Yours, Linus Walleij
Re: [PATCH v3] gpio: pl061: Support implementations without GPIOINTR line
On Mon, Mar 22, 2021 at 9:50 AM Alexander Sverdlin wrote: > On 20/03/2021 12:10, Linus Walleij wrote: > >>> I'm wondering if the GPIO library support for IRQ hierarchy is what > >>> you are looking for. > > It is indeed what should be used. > > and what has been used in my patch? Yes you're right. > > I think it can be done with quite little code. > > Guys, have you actually looked onto my patch before these reviews? I don't know why I missed it, I guess my second mail is more to the point. Yours, Linus Walleij
Re: [PATCH] mfd: ABX500_CORE should depend on ARCH_U8500
On Tue, Mar 16, 2021 at 2:37 PM Geert Uytterhoeven wrote: > The ST-Ericsson ABX500 Mixed Signal IC family chips are only present on > ST-Ericsson U8500 Series platforms. Hence add a dependency on > ARCH_U8500, to prevent asking the user about this driver when > configuring a kernel without U8500 support. > > Also, merely enabling CONFIG_COMPILE_TEST should not enable additional > code, and thus should not enable this driver by default. > > Signed-off-by: Geert Uytterhoeven Very nice. Good catch! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: GTE - The hardware timestamping engine
Hi Dipen,
thanks for your mail!
I involved some other kernel people to get some discussion.
I think Kent Gibson can be of great help because he is using
GPIOs with high precision.
We actually discussed this a bit when adding support for
realtime timestamps.
On Wed, Mar 17, 2021 at 11:29 PM Dipen Patel wrote:
> Nvidia Tegra SoCs have generic timestamping engine (GTE) hardware module which
> can monitor SoC signals like IRQ lines and GPIO lines for state change, upon
> detecting the change, it can timestamp and store in its internal hardware
> FIFO.
> The advantage of the GTE module can be realized in applications like robotics
> or autonomous vehicle where it can help record events with precise timestamp.
That sounds very useful.
Certainly the kernel shall be able to handle this.
>
> For GPIO:
>
> 1. GPIO has to be configured as input and IRQ must be enabled.
> 2. Ask GPIO controller driver to set corresponding timestamp bit in the
> specified GPIO config register.
> 3. Translate GPIO specified by the client to its internal bitmap.
> 3.a For example, If client specifies GPIO line 31, it could be bit 13 of GTE
> register.
> 4. Set internal bits to enable monitoring in GTE module
> 5. Additionally GTE driver can open up lanes for the user space application
> as a client and can send timestamping events directly to the application.
I have some concerns:
1. GPIO should for all professional applications be used with the character
device /dev/gpiochipN, under no circumstances shall the old sysfs
ABI be used for this. In this case it is necessary because the
character device provides events in a FIFO to userspace, which is
what we need.
The timestamp provided to userspace is an opaque 64bit
unsigned value. I suppose we assume it is monotonic but
you can actually augment the semantics for your specific
stamp, as long as 64 bits is gonna work.
2. The timestamp for the chardev is currently obtained in
drivers/gpio/gpiolib-cdev.c like this:
static u64 line_event_timestamp(struct line *line)
{
if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >desc->flags))
return ktime_get_real_ns();
return ktime_get_ns();
}
What you want to do is to add a new flag for hardware timestamps
and use that if available. FLAG_EVENT_CLOCK_HARDWARE?
FLAG_EVENT_CLOCK_NATIVE?
Then you need to figure out a mechanism so we can obtain
the right timestamp from the hardware event right here,
you can hook into the GPIO driver if need be, we can
figure out the gpio_chip for a certain line for sure.
So you first need to augment the userspace
ABI and the character device code to add this. See
commit 26d060e47e25f2c715a1b2c48fea391f67907a30
"gpiolib: cdev: allow edge event timestamps to be configured as REALTIME"
by Kent Gibson to see what needs to be done.
3. Also patch tools/gpio/gpio-event-mon.c to support this flag and use that
for prototyping and proof of concept.
>
> For IRQ:
>
Marc Zyngier and/or Thomas Gleixner know this stuff.
It does make sense to add some infrastructure so that GPIO events
and IRQs can use the same timestamping hardware.
And certainly you will also want to use this timestamp for
IIO devices? If it is just GPIOs and IRQs today, it will be
gyroscopes and accelerometers tomorrow, am I right?
Yours,
Linus Walleij
Re: [PATCH v7 00/22] pinctrl: add BCM63XX pincontrol support
On Wed, Mar 17, 2021 at 12:20 PM Álvaro Fernández Rojas wrote: > I appreciate that, but I’m having a hard time in understanding what > Rob wants and since there are no examples available on most of the > stuff he’s requesting this is really frustrating... I am sorry that the situation can be stressful. This is not Rob's fault, at least it's also mine. The real problem we have is lack of hardware people reviewing hardware descriptions, to put it simple. As reviewers we get a bit confused, then we try to make a mind map of the hardware as most driver developers do, but as we are not chip designers we will make mistakes and get confused and there will be a bit of back-and-forth and inconsistencies. The bindings have very high ambitions (to describe all hardware) but it's a bit like food: the less you know about how it's produced, the better the taste. In fact it is a best effort and involves a bit of guesswork and group effort and you are part of the group effort now :) Yours, Linus Walleij
Re: [PATCH v3] gpio: pl061: Support implementations without GPIOINTR line
On Fri, Mar 19, 2021 at 4:32 PM Alexander Sverdlin
wrote:
> [Andy]
> > I'm wondering if the GPIO library support for IRQ hierarchy is what
> > you are looking for.
It is indeed what should be used.
> do you have a better suggestion? That's why I reference the below discussion
> from 2017, where
> Linus Walleij suggested to use it. Well, the initial patch was just 11-liner
> and PL061 is
> rather counterexample and doesn't fit well into the existing hierarchical
> infrastructure.
>
> >> Link:
> >> https://lore.kernel.org/linux-gpio/CACRpkdZpYzpMDWqJobSYH=jhgb74hbcqihotexs+svyo6sr...@mail.gmail.com/
Don't trust that guy. He's inexperienced with the new API and talks a lot
of crap. ;)
We now have a proper API for hierarchical IRQs on GPIO controllers,
so we need to somehow detect that this is the case and act accordingly.
1. In Kconfig
select IRQ_DOMAIN_HIERARCHY if ARCH_NOKIA_FOO
2. Look at other hierarchical GPIO IRQ drivers such as
drivers/gpio/gpio-ixp4xx.c
3. Detect that we have a hierarchical situation. The hierarchical
IRQ is determined by the chip integration so I would go and
check the SoC compatible in the top-level DT, e.g.:
if (of_machine_is_compatible("nokia,rock-n-roll-soc")) {
/* Initialize the interrupt as hiearchical */
girq->fwnode =...
girq->parent_domain = ...
girq->child_to_parent_hwirq = pl061_child_to_parent_nokia_rock_n_roll;
} else {
girq->parent_handler = pl061_irq_handler;
girq->num_parents = 1;
girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
GFP_KERNEL);
if (!girq->parents)
return -ENOMEM;
girq->parents[0] = irq;
}
This might need some #ifdef or IS_ENABLED() for systems without
hierarchical IRQs.
4. Implement pl061_child_to_parent_nokia_rock_n_roll()
Just use hardcoded hardware IRQ offsets like other drivers such as
the ixp4xx does. Do not attempt to read any parent IRQs from
the device tree, and assign no IRQ in the device tree.
This is a side effect of the essentially per-soc pecularities around
interrupts. The interrupt is not cascaded so it need special
handling.
I think it can be done with quite little code.
Yours,
Linus Walleij
Re: [PATCH v7 00/22] pinctrl: add BCM63XX pincontrol support
On Mon, Mar 15, 2021 at 12:42 PM Álvaro Fernández Rojas wrote: > v7: introduce changes suggested by Rob Herring. If Rob is happy with the bindings like this (GPIO as parallel node rathern than subnode) I am ready to merge this. As long as the bindings are OK I am pretty sure any remaining nits can be fixed in-tree. Yours, Linus Walleij
Re: [PATCH v9 2/4] pinctrl: pinmux: Add pinmux-select debugfs file
On Fri, Mar 12, 2021 at 2:57 PM Enrico Weigelt, metux IT consult wrote: > On 02.03.21 06:30, Drew Fustini wrote: > > Hi folks, > > > Add "pinmux-select" to debugfs which will activate a pin function for a > > given pin group: > > > >echo "" > pinmux-select > > > > The write operation pinmux_select() handles this by checking that the > > names map to valid selectors and then calling ops->set_mux(). > > I've already been playing with similar idea, but for external muxes. > For example, some boards have multiple SIM slots that can be switched > via some gpio pin. > > Not sure whether traditional pinmux would be a good match for that. What is wrong with the subsystem drivers/mux? It's exactly for this usecase I think. Peter Rosin already wrote a GPIO-controlled mux driver too. Yours, Linus Walleij
Re: [PATCH 1/2] pinctrl: qcom: sm8350: add GPIO wakeup interrupt map
On Fri, Mar 12, 2021 at 4:41 AM Bjorn Andersson wrote: > From: Lina Iyer > > GPIOs that can be configured as wakeup sources, have their interrupt > lines routed to PDC interrupt controller. Provide the interrupt map of > the GPIO to its wakeup capable interrupt parent. > > Signed-off-by: Lina Iyer > Signed-off-by: Bjorn Andersson This patch 1/2 applied to the pin control tree. Please take the DTS patch into the qcom SoC tree! Yours, Linus Walleij
Re: [PATCH v6 04/15] dt-bindings: add BCM6328 pincontroller binding documentation
On Thu, Mar 11, 2021 at 7:14 PM Rob Herring wrote:
> > Or this way (2):
> > syscon {
> > compatible = "brcm,bcm6328-gpio-regs", "syscon", "simple-mfd";
> > reg = <0x1080 0x80>;
> > ranges = <0 0x1080 0x80>;
> >
> > pinctrl: pinctrl@18 {
> > compatible = "brcm,bcm6328-pinctrl";
> > reg = <0x0 0x28>;
> >
> > gpio: gpio@0 {
>
> This doesn't make sense IMO because GPIO is not a sub-function of the
> pinctrl h/w. They are peers.
This becomes an ontological discussion, as in "what does the world
consist of and what are the proper definitions of the
things in it".
A couple of years back I had this presentation:
https://dflund.se/~triad/papers/pincontrol.pdf
where I try to investigate how hardware engineers build
these blocks.
TL;DR: it depends on what the hardware engineer
did.
A HW block can be pin controller, GPIO controller
and interrupt chip at the same time, that case is
straight-forward. One compatible, lots of
properties.
.
A second case is when the pin controller and the
GPIO+irqchip are two completely different HW
entities, and then they also get two different
device nodes on the same level in the device tree.
(We usually see this when the different blocks
live in totally different memory locations.)
However in the third case HW can also be bolted
with a front-end pin controller (facing the pins) with
several GPIO+interrupt controller back-ends.
Then it gets the structure in this patch,
subnodes for each GPIO controller.
Our current bindings have all three examples
and it simply reflects the different ways HW
engineers have chosen to integrate their stuff.
Yours,
Linus Walleij
Re: [PATCH] pinctrl: core: Set ret to 0 when group is skipped
On Fri, Mar 12, 2021 at 8:31 AM Michal Simek wrote:
> Static analyzer tool found that the ret variable is not initialized but
> code expects ret value >=0 when pinconf is skipped in the first pinmux
> loop. The same expectation is for pinmux in a pinconf loop.
> That's why initialize ret to 0 to avoid uninitialized ret value in first
> loop or reusing ret value from first loop in second.
>
> Addresses-Coverity: ("Uninitialized variables")
> Signed-off-by: Michal Simek
> CC: Colin Ian King
> CC: Dan Carpenter
Patch applied!
Yours,
Linus Walleij
Re: [PATCH] pinctrl: ti: fix error return code of ti_iodelay_probe()
On Sat, Mar 6, 2021 at 1:51 PM Jia-Ju Bai wrote: > When ti_iodelay_pinconf_init_dev() fails, no error return code of > ti_iodelay_probe() is assigned. > To fix this bug, ret is assigned with the return value of > ti_iodelay_pinconf_init_dev(), and then ret is checked. > > Reported-by: TOTE Robot > Signed-off-by: Jia-Ju Bai Patch applied! Yours, Linus Walleij
Re: [PATCH 2/2] gpio: Add Realtek Otto GPIO support
On Mon, Mar 15, 2021 at 9:26 AM Sander Vanheule wrote:
> Realtek MIPS SoCs (platform name Otto) have GPIO controllers with up to
> 64 GPIOs, divided over two banks. Each bank has a set of registers for
> 32 GPIOs, with support for edge-triggered interrupts.
>
> Each GPIO bank consists of four 8-bit GPIO ports (ABCD and EFGH). Most
> registers pack one bit per GPIO, except for the IMR register, which
> packs two bits per GPIO (AB-CD).
>
> Although the byte order is currently assumed to have port A..D at offset
> 0x0..0x3, this has been observed to be reversed on other, Lexra-based,
> SoCs (e.g. RTL8196E/97D/97F).
>
> Interrupt support is disabled for the fallback devicetree-compatible
> 'realtek,otto-gpio'. This allows for quick support of GPIO banks in
> which the byte order would be unknown. In this case, the port ordering
> in the IMR registers may not match the reversed order in the other
> registers (DCBA, and BA-DC or DC-BA).
>
> Signed-off-by: Sander Vanheule
Overall this is a beautiful driver and it makes use of all the generic
frameworks I can think of. I don't see any reason not to merge
it so:
Reviewed-by: Linus Walleij
The following is some notes and nitpicks, nothing blocking any
merge, more like discussion.
> +enum realtek_gpio_flags {
> + GPIO_INTERRUPTS = BIT(0),
> +};
I suppose this looks like this because more flags will be introduced
when you add more functionality to the driver. Otherwise it seems
like overkill so a bool would suffice.
I would add a comment /* TODO: this will be expanded */
> +static inline u32 realtek_gpio_imr_bits(unsigned int pin, u32 value)
> +{
> + return ((value & 0x3) << 2*(pin % 16));
> +}
I would explain a bit about this, obviouslt it is two bit per
line, but it took me some time to parse, so a comment
about the bit layout would be nice.
> + unsigned int offset = pin/16;
Here that number appears again.
The use of GPIO_GENERIC and GPIO irqchip is flawless
and first class.
Thanks!
Linus Walleij
Re: [PATCH 1/2] dt-bindings: gpio: Binding for Realtek Otto GPIO
On Mon, Mar 15, 2021 at 9:25 AM Sander Vanheule wrote: > Add a binding description for Realtek's GPIO controller found on several > of their MIPS-based SoCs (codenamed Otto), such as the RTL838x and > RTL839x series of switch SoCs. > > A fallback binding 'realtek,otto-gpio' is provided for cases where the > actual port ordering is not known yet, and enabling the interrupt > controller may result in uncaught interrupts. > > Signed-off-by: Sander Vanheule These bindings look good to me. Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [RFC PATCH 1/5] rpmb: add Replay Protected Memory Block (RPMB) subsystem
On Thu, Mar 11, 2021 at 10:08 PM Alex Bennée wrote: > I guess what we are saying is that real secure monitors should come up > with their own common API for interfacing with RPMB devices without > looking to the Linux kernel for inspiration? The problem is that eMMC at least (I don't know about NVME etc) has serialized commands, meaning you execute one command at a time (some augmentation exist nowadays to speed up things). When it comes to RPMB, the eMMC device must stop all other activity (such as reading/writing any blocks to the eMMC) send a special command to switch to the RPMB partition, then execute the RPMB command(s) then send a special command to switch back to ordinary storage. Someone has to be in control of the eMMC arbitration. Right now Linux MMC/SD storage stack does this. If the secure world want to use RPMB independently of the Linux kernel there are two solutions: - Mount a second eMMC just for the secure world - looks expensive so some other secure counter storage would likely be cheaper and it inevitably increases the BOM which is sensitive to manufacturers (this option is unrealistic) - Let the secure world arbitrate all access to the eMMC - looks inefficient and also dangerous since the secure world now has to implement everything in drivers/mmc/core which is a few 100 KB of really complex code that need to be maintained perpetually. (IMO this option is also unrealistic for performance and maintenance reasons, but who knows what secure world imperialists are out there). This leaves Linux in control of the eMMC RPMB under all circumstances as far as I can see. Yours, Linus Walleij
Re: [RFC PATCH 1/5] rpmb: add Replay Protected Memory Block (RPMB) subsystem
Hi Hector, I see a misunderstanding here :) explaining below. On Thu, Mar 11, 2021 at 9:29 PM Hector Martin wrote: > And so we're back to embedded platforms like Android phones and other > SoC stuff... user-controlled secureboot is already somewhat rare here, > and even rarer are the cases where the user controls the whole chain > including the TEE if any (otherwise it'll be using RPMB already); this > pretty much excludes all production Android phones except for a few > designed as completely open systems; we're left with those and a subset > of dev boards (e.g. the Jetson TX1 I did fuse experiments on). In the > end, those systems will probably end up with fairly bespoke set-ups for > any given device or SoC family, for using RPMB. Hehe. I think we have different ideas of "user-controlled" here, our "users" include OP-TEE, which develop and deploy a TEE which is open source. https://www.op-tee.org/ Joakim who works on this project is on CC he's just not saying anything (yet). This project is forked and deployed by different Android and other Arm SoC-using vendors. Some vendors have written their own TEE from scratch. So our users include these guys. :) As in: they take an active interest in what we are designing here. They have access to devices where they can replace the whole secure world for development. They work actively with the kernel and created the drivers/tee subsystem which is the pipe where the kernel and the TEE communicate. > But then again, if you have a full secureboot system where you control > the TEE level, wouldn't you want to put the RPMB shenanigans there and > get some semblance of secure TPM/keystore/attempt throttling > functionality that is robust against Linux exploits and has a smaller > attack surface? Systems without EL3 are rare (Apple M1 :-)) so it makes > more sense to do this on those that do have it. If you're paranoid > enough to be getting into building your own secure system with > anti-rollback for retry counters, you should be heading in that directly > anyway. > > And now Linux's RPMB code is useless because you're running the stack in > the secure monitor instead :-) The way OP-TEE makes use of RPMB is to call out to a userspace daemon called tee-supplicant, which issues ioctl()s down to the eMMC device to read/write counters. AFAIK other TEE implementations use a similar scheme. (Judging from feedback I got when rewriting the RPMB code in the MMC subsystem, it mattered to them.) Their source code is here: https://github.com/OP-TEE/optee_client/blob/master/tee-supplicant/src/rpmb.c So Linux' eMMC RPMB code is already in wide use for this, it is what I think all Android phones are actually using to read/write RPMB counters. It's not like they're accessing RPMB "on the side" or so. (I might be wrong!) Since reading/writing RPMB on eMMC needs to be serialized alongside Linux' read/write commands it is absolutely necessary that the secure world and the Linux storage drivers cooperate so the solution is to let Linux handle this arbitration. Now the question for this patch set is: is TEE software the only user we need to care about? Yours, Linus Walleij
Re: [RFC PATCH 1/5] rpmb: add Replay Protected Memory Block (RPMB) subsystem
Hi Hector, thanks for the long and detailed answer! I learn new things all the time. (Maybe one day I add something too, who knows.) I hope I'm not taking too much of your time, we're having fun :) On Thu, Mar 11, 2021 at 9:02 PM Hector Martin wrote: > On 11/03/2021 23.06, Linus Walleij wrote: > > Yes. And this is what mobile phone vendors typically did. > > > > But the nature of different electrical attacks made them worried > > about different schemes involving cutting power and disturbing > > signals with different probes, so they wanted this counter > > implemented in hardware and that is why RPMB exists at all > > (IIUC). > > No, prior to RPMB there was no such secure counter at all. The problem > is that non-volatile erasable storage (i.e. EEPROM/Flash) is > incompatible with modern SoC manufacturing processes, so there is no way > to embed a secure counter into the main SoC. And once your counter needs > to be external, there needs to be a secure communications protocol to > access it. This is what RPMB implements. > > For preventing software downgrades, especially of bootloader code, this > can be implemented with one-time fuses embedded in the SoC, but there is > a limited supply of those. So this doesn't work for things like PIN > attempt counters. For that you need a secure external counter. Actually what we did (I was there, kind of) was to go to the flash vendors (IIRC first Intel) and require what is today called "fuses" in the flash memory. Originally this was for things like unique serial numbers set in production. But they could easily add some more of it for other use cases. This became what is known as OTP (one time programmable flash). The OTP was all set to 1:s when the flash was new, then what we did for anti-rollback was to designate some bits for software versions. To make sure the OTP readout wasn't tampered with, some additional hashes of the OTP was stored in the flash and MAC signed. This was recalculated when we changed a bit from 1->0 in the OTP to indicate a new firmware version. Clever, isn't it? :) I think the scheme in RPMB was based in part on the needs solved by that crude mechanism. (Linux MTD did actually even gain some support for OTP recently, it is used only from userspace AFIAK.) > RPMB isn't pointless; what I am saying is that > if you strip away everything but the counter functionality, you can > still build equivalent security guarantees. You still need the counter. > There is no way to get that counter without RPMB or something like it > (another option is e.g. to use a smartcard IC as a secure element; AIUI > modern Apple devices do this). Software alone doesn't work. This is why > I wrote that article about how the FBI cracks iPhones; that works > because they weren't using a secure rollback-protected storage/counter > chip of any kind. Yeah. Hm, actually if they had flash memory they should have used the OTP... But I suppose they were all on eMMC. > it helps if you forget about the read/write commands and treat > it as a simple counter. Yep you're right. > Once you do that, you'll realize that e.g. putting keys in RPMB doesn't > really make sense as a kernel primitive. The usefulness of RPMB is > purely in the integration of that counter (which is equivalent to > rollback-protected storage) with a policy system. Everything else is > icing on the cake; it doesn't create new use cases. OK I understand. So what you're saying is we can't develop anything without also developing a full policy system. > Consider this: (...) > You have now built a secure, rollback-protected Git repository, with > similar security properties to RPMB storage, without using RPMB storage; > just a counter. This example of using the RPMB to protect rollback of a git was really nice! I think I understood as much before but maybe I don't explain that well enough :/ > Thus, we can conclude that the storage features of RPMB do not provide > additional security properties that cannot be derived from a simple counter. I agree. > * Disclaimer: please don't actually deploy this; I'm trying to make a > point here, it's 5AM and I'm not claiming this is a perfectly secure > design and I haven't missed anything. Please don't design > rollback-protected Git repositories without expert review. I am assuming > filesystem mutations only happen between operations and handwaving away > active attacks, which I doubt Git is designed to be robust against. A > scheme like this can be implemented securely with care, but not naively. It's an example all kernel developers can relate to, so the educational value is high! > Well, that's what I'm saying, you do need secureboot for this to make > sense :-) > > RPMB isn't useless and some systems should implement it; but there's no > real way f
Re: [PATCH 0/4] mfd/power: Push data into power supply
On Fri, Mar 12, 2021 at 9:36 AM Linus Walleij wrote: > - The power maintainer (Sebastian) provide an ACK Ooops I noticed actuall Sebastian already gave an ACK for these patches: https://lore.kernel.org/linux-pm/20210128001700.pkuyfpq6uzcjb5ud@earth.universe/ Sorry for keeping bad track. This means Lee can add Sebastians ACK and merge these patches at will. (I can also resend with the ACKs if need be.) Yours, Linus Walleij
[PATCH 4/4] mfd/power: ab8500: Push data to power supply code
There is a slew of defines, structs and enums and even a
function call only relevant for the charging code that
still lives in . Push it down to the
"ab8500-bm.h" header in the power supply subsystem where
it is actually used.
Signed-off-by: Linus Walleij
---
drivers/power/supply/ab8500-bm.h | 278 ++-
include/linux/mfd/abx500.h | 276 --
2 files changed, 274 insertions(+), 280 deletions(-)
diff --git a/drivers/power/supply/ab8500-bm.h b/drivers/power/supply/ab8500-bm.h
index a1b31c971a45..41c69a4f2a1f 100644
--- a/drivers/power/supply/ab8500-bm.h
+++ b/drivers/power/supply/ab8500-bm.h
@@ -4,7 +4,6 @@
#define _AB8500_CHARGER_H_
#include
-#include
/*
* System control 2 register offsets.
@@ -268,6 +267,277 @@ enum bup_vch_sel {
#define BUS_PP_PRECHG_CURRENT_MASK 0x0E
#define BUS_POWER_PATH_PRECHG_ENA 0x01
+/*
+ * ADC for the battery thermistor.
+ * When using the ABx500_ADC_THERM_BATCTRL the battery ID resistor is combined
+ * with a NTC resistor to both identify the battery and to measure its
+ * temperature. Different phone manufactures uses different techniques to both
+ * identify the battery and to read its temperature.
+ */
+enum abx500_adc_therm {
+ ABx500_ADC_THERM_BATCTRL,
+ ABx500_ADC_THERM_BATTEMP,
+};
+
+/**
+ * struct abx500_res_to_temp - defines one point in a temp to res curve. To
+ * be used in battery packs that combines the identification resistor with a
+ * NTC resistor.
+ * @temp: battery pack temperature in Celsius
+ * @resist:NTC resistor net total resistance
+ */
+struct abx500_res_to_temp {
+ int temp;
+ int resist;
+};
+
+/**
+ * struct abx500_v_to_cap - Table for translating voltage to capacity
+ * @voltage: Voltage in mV
+ * @capacity: Capacity in percent
+ */
+struct abx500_v_to_cap {
+ int voltage;
+ int capacity;
+};
+
+/* Forward declaration */
+struct abx500_fg;
+
+/**
+ * struct abx500_fg_parameters - Fuel gauge algorithm parameters, in seconds
+ * if not specified
+ * @recovery_sleep_timer: Time between measurements while recovering
+ * @recovery_total_time: Total recovery time
+ * @init_timer:Measurement interval during startup
+ * @init_discard_time: Time we discard voltage measurement at startup
+ * @init_total_time: Total init time during startup
+ * @high_curr_time:Time current has to be high to go to recovery
+ * @accu_charging: FG accumulation time while charging
+ * @accu_high_curr:FG accumulation time in high current mode
+ * @high_curr_threshold: High current threshold, in mA
+ * @lowbat_threshold: Low battery threshold, in mV
+ * @overbat_threshold: Over battery threshold, in mV
+ * @battok_falling_th_sel0 Threshold in mV for battOk signal sel0
+ * Resolution in 50 mV step.
+ * @battok_raising_th_sel1 Threshold in mV for battOk signal sel1
+ * Resolution in 50 mV step.
+ * @user_cap_limit Capacity reported from user must be within this
+ * limit to be considered as sane, in percentage
+ * points.
+ * @maint_thresThis is the threshold where we stop
reporting
+ * battery full while in maintenance, in per cent
+ * @pcut_enable: Enable power cut feature in ab8505
+ * @pcut_max_time: Max time threshold
+ * @pcut_flag_time:Flagtime threshold
+ * @pcut_max_restart: Max number of restarts
+ * @pcut_debounce_time:Sets battery debounce time
+ */
+struct abx500_fg_parameters {
+ int recovery_sleep_timer;
+ int recovery_total_time;
+ int init_timer;
+ int init_discard_time;
+ int init_total_time;
+ int high_curr_time;
+ int accu_charging;
+ int accu_high_curr;
+ int high_curr_threshold;
+ int lowbat_threshold;
+ int overbat_threshold;
+ int battok_falling_th_sel0;
+ int battok_raising_th_sel1;
+ int user_cap_limit;
+ int maint_thres;
+ bool pcut_enable;
+ u8 pcut_max_time;
+ u8 pcut_flag_time;
+ u8 pcut_max_restart;
+ u8 pcut_debounce_time;
+};
+
+/**
+ * struct abx500_charger_maximization - struct used by the board config.
+ * @use_maxi: Enable maximization for this battery type
+ * @maxi_chg_curr: Maximum charger current allowed
+ * @maxi_wait_cycles: cycles to wait before setting charger current
+ * @charger_curr_step delta between two charger current settings (mA)
+ */
+struct abx500_maxim_parameters {
+ bool ena_maxi;
+ int chg_curr;
+ int wait_cycles;
+ int charger_curr_step;
+};
+
+/**
+ * struct abx500_battery_type - different batteries
[PATCH 0/4] mfd/power: Push data into power supply
This series pushes some AB8500 power supply headers down
into the power supply subsystem so the power supply code
becomes independent from the other AB8500 stuff.
The first patch makes the code require device tree so
that the series make sense: once all data for the power
supply comes from device tree, it makes sense for that
code to not require global headers for platform data
etc.
This is in preparation for some finalization of the AB8500
power code, as merge strategy I think it is best if:
- The power maintainer (Sebastian) provide an ACK
- The MFD matinainer (Lee) merges this and provide an
immutable branch that the power maintainer can possibly
pull as a base for his tree
I hope both subsystems are happy with the changes.
One of the patches already has Lee's Acked-for-MFD, but I
got a bit stressed out in the last kernel cycle. Let's
take this stepwise, first these four patches. No hurry.
Linus Walleij (4):
mfd/power: ab8500: Require device tree
mfd/power: ab8500: Push data to power supply code
mfd/power: ab8500: Push algorithm to power supply code
mfd/power: ab8500: Push data to power supply code
drivers/mfd/ab8500-core.c | 17 +-
drivers/power/supply/Kconfig | 2 +-
.../power/supply}/ab8500-bm.h | 297 --
.../power/supply/ab8500-chargalg.h| 6 +-
drivers/power/supply/ab8500_bmdata.c | 3 +-
drivers/power/supply/ab8500_btemp.c | 45 +--
drivers/power/supply/ab8500_charger.c | 27 +-
drivers/power/supply/ab8500_fg.c | 20 +-
drivers/power/supply/abx500_chargalg.c| 22 +-
drivers/power/supply/pm2301_charger.c | 4 +-
include/linux/mfd/abx500.h| 276
11 files changed, 326 insertions(+), 393 deletions(-)
rename {include/linux/mfd/abx500 => drivers/power/supply}/ab8500-bm.h (58%)
rename include/linux/mfd/abx500/ux500_chargalg.h =>
drivers/power/supply/ab8500-chargalg.h (93%)
--
2.29.2
[PATCH 2/4] mfd/power: ab8500: Push data to power supply code
The global definition of platform data for the battery
management code has no utility after the OF conversion,
move the to be a local
file in drivers/power/supply and stop defining the
platform data in drivers/power/supply/ab8500_bmdata.c
and broadcast to the kernel only to have it assigned
as platform data to the MFD cells and then picked back
into the same subsystem that defined it in the first
place. This kills off a layer of indirection.
Acked-for-MFD-by: Lee Jones
Signed-off-by: Linus Walleij
---
drivers/mfd/ab8500-core.c | 17 +
.../power/supply}/ab8500-bm.h | 19 ++
drivers/power/supply/ab8500_bmdata.c | 3 +-
drivers/power/supply/ab8500_btemp.c | 35 +++
drivers/power/supply/ab8500_charger.c | 10 ++
drivers/power/supply/ab8500_fg.c | 10 ++
drivers/power/supply/abx500_chargalg.c| 10 ++
drivers/power/supply/pm2301_charger.c | 2 +-
8 files changed, 27 insertions(+), 79 deletions(-)
rename {include/linux/mfd/abx500 => drivers/power/supply}/ab8500-bm.h (96%)
diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index 2dde3a7532c4..7bb23c0d1efd 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -19,7 +19,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -603,14 +602,14 @@ int ab8500_suspend(struct ab8500 *ab8500)
}
static const struct mfd_cell ab8500_bm_devs[] = {
- MFD_CELL_OF("ab8500-charger", NULL, _bm_data,
- sizeof(ab8500_bm_data), 0, "stericsson,ab8500-charger"),
- MFD_CELL_OF("ab8500-btemp", NULL, _bm_data,
- sizeof(ab8500_bm_data), 0, "stericsson,ab8500-btemp"),
- MFD_CELL_OF("ab8500-fg", NULL, _bm_data,
- sizeof(ab8500_bm_data), 0, "stericsson,ab8500-fg"),
- MFD_CELL_OF("ab8500-chargalg", NULL, _bm_data,
- sizeof(ab8500_bm_data), 0, "stericsson,ab8500-chargalg"),
+ MFD_CELL_OF("ab8500-charger", NULL, NULL, 0, 0,
+ "stericsson,ab8500-charger"),
+ MFD_CELL_OF("ab8500-btemp", NULL, NULL, 0, 0,
+ "stericsson,ab8500-btemp"),
+ MFD_CELL_OF("ab8500-fg", NULL, NULL, 0, 0,
+ "stericsson,ab8500-fg"),
+ MFD_CELL_OF("ab8500-chargalg", NULL, NULL, 0, 0,
+ "stericsson,ab8500-chargalg"),
};
static const struct mfd_cell ab8500_devs[] = {
diff --git a/include/linux/mfd/abx500/ab8500-bm.h
b/drivers/power/supply/ab8500-bm.h
similarity index 96%
rename from include/linux/mfd/abx500/ab8500-bm.h
rename to drivers/power/supply/ab8500-bm.h
index 903e94c189d8..a1b31c971a45 100644
--- a/include/linux/mfd/abx500/ab8500-bm.h
+++ b/drivers/power/supply/ab8500-bm.h
@@ -1,12 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright ST-Ericsson 2012.
- *
- * Author: Arun Murthy
- */
-#ifndef _AB8500_BM_H
-#define _AB8500_BM_H
+#ifndef _AB8500_CHARGER_H_
+#define _AB8500_CHARGER_H_
#include
#include
@@ -453,16 +448,11 @@ struct ab8500_bm_data {
};
struct ab8500_btemp;
-struct ab8500_gpadc;
struct ab8500_fg;
-#ifdef CONFIG_AB8500_BM
extern struct abx500_bm_data ab8500_bm_data;
void ab8500_charger_usb_state_changed(u8 bm_usb_state, u16 mA);
-struct ab8500_btemp *ab8500_btemp_get(void);
-int ab8500_btemp_get_batctrl_temp(struct ab8500_btemp *btemp);
-int ab8500_btemp_get_temp(struct ab8500_btemp *btemp);
struct ab8500_fg *ab8500_fg_get(void);
int ab8500_fg_inst_curr_blocking(struct ab8500_fg *dev);
int ab8500_fg_inst_curr_start(struct ab8500_fg *di);
@@ -470,7 +460,4 @@ int ab8500_fg_inst_curr_finalize(struct ab8500_fg *di, int
*res);
int ab8500_fg_inst_curr_started(struct ab8500_fg *di);
int ab8500_fg_inst_curr_done(struct ab8500_fg *di);
-#else
-static struct abx500_bm_data ab8500_bm_data;
-#endif
-#endif /* _AB8500_BM_H */
+#endif /* _AB8500_CHARGER_H_ */
diff --git a/drivers/power/supply/ab8500_bmdata.c
b/drivers/power/supply/ab8500_bmdata.c
index f6a66979cbb5..c2b8c0bb77e2 100644
--- a/drivers/power/supply/ab8500_bmdata.c
+++ b/drivers/power/supply/ab8500_bmdata.c
@@ -4,7 +4,8 @@
#include
#include
#include
-#include
+
+#include "ab8500-bm.h"
/*
* These are the defined batteries that uses a NTC and ID resistor placed
diff --git a/drivers/power/supply/ab8500_btemp.c
b/drivers/power/supply/ab8500_btemp.c
index ca5153c82c81..33fb6f65749c 100644
--- a/drivers/power/supply/ab8500_btemp.c
+++ b/drivers/power/supply/ab8500_btemp.c
@@ -25,9 +25,10 @@
#include
#include
#include
-#include
#include
+#include "ab8500-bm.h"
+
#define VTVOUT_V 1800
#define BTEMP_THERMAL_LOW_LIMIT-10
@@ -119,16 +120,6 @@ static enum power_supply_property ab8
[PATCH 3/4] mfd/power: ab8500: Push algorithm to power supply code
The charging algorithm header is only used locally in the
power supply subsystem so push this down into
drivers/power/supply and rename from the confusing
"ux500_chargalg.h" to "ab8500-chargalg.h" for clarity:
it is only used with the AB8500.
This is another remnant of non-DT code needing to pass
data from boardfiles, which we don't do anymore.
Signed-off-by: Linus Walleij
---
.../power/supply/ab8500-chargalg.h | 6 +++---
drivers/power/supply/ab8500_charger.c | 2 +-
drivers/power/supply/abx500_chargalg.c | 2 +-
drivers/power/supply/pm2301_charger.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
rename include/linux/mfd/abx500/ux500_chargalg.h =>
drivers/power/supply/ab8500-chargalg.h (93%)
diff --git a/include/linux/mfd/abx500/ux500_chargalg.h
b/drivers/power/supply/ab8500-chargalg.h
similarity index 93%
rename from include/linux/mfd/abx500/ux500_chargalg.h
rename to drivers/power/supply/ab8500-chargalg.h
index 9b97d284d0ce..94a6f9068bc5 100644
--- a/include/linux/mfd/abx500/ux500_chargalg.h
+++ b/drivers/power/supply/ab8500-chargalg.h
@@ -4,8 +4,8 @@
* Author: Johan Gardsmark for ST-Ericsson.
*/
-#ifndef _UX500_CHARGALG_H
-#define _UX500_CHARGALG_H
+#ifndef _AB8500_CHARGALG_H_
+#define _AB8500_CHARGALG_H_
#include
@@ -48,4 +48,4 @@ struct ux500_charger {
extern struct blocking_notifier_head charger_notifier_list;
-#endif
+#endif /* _AB8500_CHARGALG_H_ */
diff --git a/drivers/power/supply/ab8500_charger.c
b/drivers/power/supply/ab8500_charger.c
index 50989a5ec95c..a9be10eb2c22 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -28,12 +28,12 @@
#include
#include
#include
-#include
#include
#include
#include
#include "ab8500-bm.h"
+#include "ab8500-chargalg.h"
/* Charger constants */
#define NO_PW_CONN 0
diff --git a/drivers/power/supply/abx500_chargalg.c
b/drivers/power/supply/abx500_chargalg.c
index 5b28d58041b4..f5b792243727 100644
--- a/drivers/power/supply/abx500_chargalg.c
+++ b/drivers/power/supply/abx500_chargalg.c
@@ -28,10 +28,10 @@
#include
#include
#include
-#include
#include
#include "ab8500-bm.h"
+#include "ab8500-chargalg.h"
/* Watchdog kick interval */
#define CHG_WD_INTERVAL(6 * HZ)
diff --git a/drivers/power/supply/pm2301_charger.c
b/drivers/power/supply/pm2301_charger.c
index 5aeff75db33b..d53e0c37c059 100644
--- a/drivers/power/supply/pm2301_charger.c
+++ b/drivers/power/supply/pm2301_charger.c
@@ -18,13 +18,13 @@
#include
#include
#include
-#include
#include
#include
#include
#include
#include "ab8500-bm.h"
+#include "ab8500-chargalg.h"
#include "pm2301_charger.h"
#define to_pm2xxx_charger_ac_device_info(x) container_of((x), \
--
2.29.2
[PATCH 1/4] mfd/power: ab8500: Require device tree
The core AB8500 driver and the whole platform is completely
dependent on being probed from device tree so remove the
non-DT probe paths.
Signed-off-by: Linus Walleij
---
drivers/power/supply/Kconfig | 2 +-
drivers/power/supply/ab8500_btemp.c| 10 --
drivers/power/supply/ab8500_charger.c | 15 ++-
drivers/power/supply/ab8500_fg.c | 10 --
drivers/power/supply/abx500_chargalg.c | 10 --
5 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 006b95eca673..a910571e8d4f 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -698,7 +698,7 @@ config BATTERY_GAUGE_LTC2941
config AB8500_BM
bool "AB8500 Battery Management Driver"
- depends on AB8500_CORE && AB8500_GPADC && (IIO = y)
+ depends on AB8500_CORE && AB8500_GPADC && (IIO = y) && OF
help
Say Y to include support for AB8500 battery management.
diff --git a/drivers/power/supply/ab8500_btemp.c
b/drivers/power/supply/ab8500_btemp.c
index 7095ea4c68d6..ca5153c82c81 100644
--- a/drivers/power/supply/ab8500_btemp.c
+++ b/drivers/power/supply/ab8500_btemp.c
@@ -1008,12 +1008,10 @@ static int ab8500_btemp_probe(struct platform_device
*pdev)
}
di->bm = plat;
- if (np) {
- ret = ab8500_bm_of_probe(dev, np, di->bm);
- if (ret) {
- dev_err(dev, "failed to get battery information\n");
- return ret;
- }
+ ret = ab8500_bm_of_probe(dev, np, di->bm);
+ if (ret) {
+ dev_err(dev, "failed to get battery information\n");
+ return ret;
}
/* get parent data */
diff --git a/drivers/power/supply/ab8500_charger.c
b/drivers/power/supply/ab8500_charger.c
index ac77c8882d17..aa573cd299e2 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -3360,15 +3360,12 @@ static int ab8500_charger_probe(struct platform_device
*pdev)
}
di->bm = plat;
- if (np) {
- ret = ab8500_bm_of_probe(dev, np, di->bm);
- if (ret) {
- dev_err(dev, "failed to get battery information\n");
- return ret;
- }
- di->autopower_cfg = of_property_read_bool(np, "autopower_cfg");
- } else
- di->autopower_cfg = false;
+ ret = ab8500_bm_of_probe(dev, np, di->bm);
+ if (ret) {
+ dev_err(dev, "failed to get battery information\n");
+ return ret;
+ }
+ di->autopower_cfg = of_property_read_bool(np, "autopower_cfg");
/* get parent data */
di->dev = dev;
diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index 06ff42c71f24..079e11325a81 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -3043,12 +3043,10 @@ static int ab8500_fg_probe(struct platform_device *pdev)
}
di->bm = plat;
- if (np) {
- ret = ab8500_bm_of_probe(dev, np, di->bm);
- if (ret) {
- dev_err(dev, "failed to get battery information\n");
- return ret;
- }
+ ret = ab8500_bm_of_probe(dev, np, di->bm);
+ if (ret) {
+ dev_err(dev, "failed to get battery information\n");
+ return ret;
}
mutex_init(>cc_lock);
diff --git a/drivers/power/supply/abx500_chargalg.c
b/drivers/power/supply/abx500_chargalg.c
index a9d84d845f24..591ddd2987a3 100644
--- a/drivers/power/supply/abx500_chargalg.c
+++ b/drivers/power/supply/abx500_chargalg.c
@@ -1997,12 +1997,10 @@ static int abx500_chargalg_probe(struct platform_device
*pdev)
}
di->bm = plat;
- if (np) {
- ret = ab8500_bm_of_probe(>dev, np, di->bm);
- if (ret) {
- dev_err(>dev, "failed to get battery
information\n");
- return ret;
- }
+ ret = ab8500_bm_of_probe(>dev, np, di->bm);
+ if (ret) {
+ dev_err(>dev, "failed to get battery information\n");
+ return ret;
}
/* get device struct and parent */
--
2.29.2
Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1
On Thu, Mar 11, 2021 at 2:01 AM Doug Anderson wrote: > If you happen to feel in an applying mood one other patch to > simple-panel I think is OK to land is at: > > https://lore.kernel.org/r/20210222081716.1.I1a45aece5d2ac6a2e73bbec50da2086e43e0862b@changeid I applied and pushed this as well. Yours, Linus Walleij
Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1
On Fri, Jan 15, 2021 at 11:44 PM Douglas Anderson wrote: > This series is to get the N116BCA-EA1 panel working. Most of the > patches are simple, but on hardware I have in front of me the panel > sometimes doesn't come up. I'm still working with the hardware > manufacturer to get to the bottom of it, but I've got it working with > retries. Adding the retries doesn't seem like an insane thing to do > and makes some of the error handling more robust, so I've gone ahead > and included those patches here. Hopefully they look OK. > > Changes in v2: This v2 version applied to drm-misc-next. Yours, Linus Walleij
Re: [PATCH v6 03/15] pinctrl: bcm: add bcm63xx base code
On Thu, Mar 11, 2021 at 3:58 PM Rob Herring wrote:
> On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij
> wrote:
> > On Wed, Mar 10, 2021 at 6:51 PM Rob Herring wrote:
> >
> > > > +static const struct of_device_id bcm63xx_gpio_of_match[] = {
> > > > + { .compatible = "brcm,bcm6318-gpio", },
> > > > + { .compatible = "brcm,bcm6328-gpio", },
> > > > + { .compatible = "brcm,bcm6358-gpio", },
> > > > + { .compatible = "brcm,bcm6362-gpio", },
> > > > + { .compatible = "brcm,bcm6368-gpio", },
> > > > + { .compatible = "brcm,bcm63268-gpio", },
> > >
> > > All these would be moved to gpio-mmio.c (or maybe that can have a
> > > fallback compatible?).
> >
> > This is gpio-regmap.c and it can only be used as a library
> > by a certain driver. gpio-mmio.c can be used stand-alone
> > for certain really simple hardware (though most use that
> > as a library as well).
>
> I don't really care which one is used, but the problem is that this
> choice is leaking into the binding design.
Aha I guess I misunderstood your comment.
>The primary problem here is
> once someone uses regmap, then they think they must have a syscon and
> can abandon using 'reg' and normal address properties as Linux happens
> to not use them (currently). I think we really need some better regmap
> vs. mmio handling to eliminate this duplication of foo-mmio and
> foo-regmap drivers and difference in binding design. Not sure exactly
> what that looks like, but basically some sort of 'reg' property to
> regmap creation.
I see the problem. Yeah we should try to be more strict around
these things. To me there are syscons and "other regmaps",
where syscon is a real hurdle of registers while "other regmaps"
are just regmaps by convenience.
Documentation/devicetree/bindings/mfd/syscon.yaml
describes what a syscon really is so if everyone could
just read the documentation that would be great ...
> Given we already have a Broadcom GPIO binding for what looks to be
> similar to this one, I'm left wondering what's the real difference
> here?
Which one is similar? I can take a look.
We currently have four Broadcom GPIO bindings,
which are stand alone GPIO blocks and eight Broadcom
pin controllers that all do GPIO as well.
This family of pin controllers are (as per subject) is
the bcm63xx series which is a MIPS-based family of SoCs
found in routers, top bindings in
Documentation/devicetree/bindings/mips/brcm/soc.txt
These all have a GPIO block as part of the pin controller
and the GPIO block is a distinct sub-function of the
pin controller, and it has up to 32 GPIOs per block,
hence it has its own subnode inside the pin controller.
This driver follows the pattern of the Ingenic
pin controller, another MIPS SoC:
Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml
Another SoC with several GPIO blocks inside the pin
controller is SparX5 and that also follows this pattern:
Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml
(This has an example with more than one GPIO block
inside the pin controller.)
Yours,
Linus Walleij
