* Re: [PATCH] arm64: dts: qcom: ipq8074: Remove unused gpio from QPIC pins
From: Bjorn Andersson @ 2024-04-04 21:22 UTC (permalink / raw)
To: Paweł Owoc
Cc: Robert Marko, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240313102713.1727458-1-frut3k7@gmail.com>
On Wed, 13 Mar 2024 11:27:06 +0100, Paweł Owoc wrote:
> gpio16 will only be used for LCD support, as its NAND/LCDC data[8]
> so its bit 9 of the parallel QPIC interface, and ONFI NAND is only 8
> or 16-bit with only 8-bit one being supported in our case so that pin
> is unused.
>
> It should be dropped from the default NAND pinctrl configuration
> as its unused and only needed for LCD.
>
> [...]
Applied, thanks!
[1/1] arm64: dts: qcom: ipq8074: Remove unused gpio from QPIC pins
commit: 5f78d9213ae753e2242b0f6a5d4a5e98e55ddc76
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: msm8998-yoshino: Enable RGB led
From: Bjorn Andersson @ 2024-04-04 21:22 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel,
Sebastian Raase, Konrad Dybcio
In-Reply-To: <20240316-topic-maple_led-v1-1-ca3430fd9dc5@linaro.org>
On Sat, 16 Mar 2024 13:10:46 +0100, Konrad Dybcio wrote:
> Add the multicolor description and enable the PMI8998 LPG to expose the
> RGB notification LED.
>
>
Applied, thanks!
[1/1] arm64: dts: qcom: msm8998-yoshino: Enable RGB led
commit: 7c2a774f028f6e2acc40bf969fcac288d3143c7f
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: apq8016-sbc: correct GPIO LEDs node names
From: Bjorn Andersson @ 2024-04-04 21:22 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-arm-msm, devicetree, linux-kernel, Krzysztof Kozlowski
Cc: Sumit Garg
In-Reply-To: <20240314112657.167006-1-krzysztof.kozlowski@linaro.org>
On Thu, 14 Mar 2024 12:26:57 +0100, Krzysztof Kozlowski wrote:
> Individual LEDs in a GPIO LEDs device node are not addressable, thus
> unit address is not correct.
>
> dtc is also not happy:
>
> Warning (unit_address_vs_reg): /leds/led@5: node has a unit name, but no reg or ranges property
>
> [...]
Applied, thanks!
[1/1] arm64: dts: qcom: apq8016-sbc: correct GPIO LEDs node names
commit: 216e62744b91c9716228fe13f564e83381a1342e
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Bjorn Andersson @ 2024-04-04 21:22 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Dmitry Baryshkov, Danila Tikhonov,
linux-arm-msm, devicetree, linux-kernel, Krzysztof Kozlowski
In-Reply-To: <20240328074544.5076-1-krzysztof.kozlowski@linaro.org>
On Thu, 28 Mar 2024 08:45:44 +0100, Krzysztof Kozlowski wrote:
> The first part of the compatible of Type-C node misses ending quote,
> thus we have one long compatible consisting of two compatible strings
> leading to dtbs_check warnings:
>
> sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
> sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
>
> [...]
Applied, thanks!
[1/1] arm64: dts: qcom: pm6150: correct Type-C compatible
commit: 5582e357d0c6bfdc889773ca3c9b7b0dd31dc334
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: msm8998-yoshino: fix volume-up key
From: Bjorn Andersson @ 2024-04-04 21:22 UTC (permalink / raw)
To: Sebastian Raase
Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
AngeloGioacchino Del Regno, Marijn Suijten, linux-arm-msm,
devicetree, linux-kernel
In-Reply-To: <20240315225237.1616550-1-linux@sraa.de>
On Fri, 15 Mar 2024 23:52:29 +0100, Sebastian Raase wrote:
> The volume-up key is connected to gpio6 on yoshino.
> Fix button node ordering while at it.
> Disable pm8998_resin, since it is now unused.
>
> Tested on maple and lilac.
>
>
> [...]
Applied, thanks!
[1/1] arm64: dts: qcom: msm8998-yoshino: fix volume-up key
commit: 83ef6a5afc1d5e2270797a164972a3de3bd2ea52
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH v3] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: Sebastian Raase
Cc: marijn.suijten, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240315085934.1511722-1-linux@sraa.de>
On Fri, 15 Mar 2024 09:59:25 +0100, Sebastian Raase wrote:
> Add pinctrl configuration for gpio-keys. Without this,
> camera button half-presses are not detected.
>
> Tested on discovery and pioneer.
>
>
Applied, thanks!
[1/1] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key
commit: 0fba148c3ac0fb8bc3d2a788c19b2ede4e5d3695
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: (subset) [PATCH 0/2] Add Inline Crypto Engine for SC7280 UFS
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: Konrad Dybcio, Herbert Xu, David S. Miller, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, cros-qcom-dts-watchers,
Luca Weiss
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm,
linux-crypto, devicetree, linux-kernel
In-Reply-To: <20240313-sc7280-ice-v1-0-3fa089fb7a27@fairphone.com>
On Wed, 13 Mar 2024 13:53:13 +0100, Luca Weiss wrote:
> Add the required bits to support Inline Crypto Engine on SC7280 SoC with
> UFS.
>
>
Applied, thanks!
[2/2] arm64: dts: qcom: sc7280: Add inline crypto engine
commit: dfd5ee7b34bb7611d4d2f4f3cb37152baeaae96d
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: (subset) [PATCH v2 0/3] Split sony-castor into shinano-common and add Sony Xperia Z3
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: ~postmarketos/upstreaming, phone-devel, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Luca Weiss
Cc: linux-arm-msm, devicetree, linux-kernel, Krzysztof Kozlowski
In-Reply-To: <20240314-shinano-common-v2-0-a0fce1c72c74@z3ntu.xyz>
On Thu, 14 Mar 2024 19:56:21 +0100, Luca Weiss wrote:
> Prepare for adding sony-leo dts by splitting common parts into a
> separate dtsi file.
>
> Then add the dts for Sony Xperia Z3.
>
> Depends on:
> https://lore.kernel.org/linux-arm-msm/20240306-castor-changes-v1-0-2286eaf85fff@z3ntu.xyz/T/
>
> [...]
Applied, thanks!
[1/3] ARM: dts: qcom: msm8974-sony-castor: Split into shinano-common
commit: 53426f53eda5e4a17197a8bc7dd1045601db407e
[3/3] ARM: dts: qcom: Add Sony Xperia Z3 smartphone
commit: 8d91a5a4a6f5aff714a14ac4a86931aa789655d8
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: sm8650: fix usb interrupts properties
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong
Cc: linux-arm-msm, devicetree, linux-kernel, Krishna Kurapati
In-Reply-To: <20240314-topic-sm8650-upstream-usb-dt-irq-fix-v1-1-ea8ab2051869@linaro.org>
On Thu, 14 Mar 2024 09:53:06 +0100, Neil Armstrong wrote:
> Update the usb interrupts properties to fix the following
> bindings check errors:
> usb@a6f8800: interrupt-names:0: 'pwr_event' was expected
> from schema $id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
> usb@a6f8800: interrupt-names:1: 'hs_phy_irq' was expected
> from schema $id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
> usb@a6f8800: interrupt-names:2: 'dp_hs_phy_irq' was expected
> from schema $id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
> usb@a6f8800: interrupt-names:3: 'dm_hs_phy_irq' was expected
> from schema $id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
> usb@a6f8800: interrupt-names: ['hs_phy_irq', 'ss_phy_irq', 'dm_hs_phy_irq', 'dp_hs_phy_irq'] is too short
> from schema $id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
>
> [...]
Applied, thanks!
[1/1] arm64: dts: qcom: sm8650: fix usb interrupts properties
commit: 9f42f7380f6757ce7f0cab5bad56fb350941d32b
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: (subset) [PATCH v2 0/6] arm64: dts: qcom: qcs6490-rb3gen2: Enable two displays
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: cros-qcom-dts-watchers, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas, Will Deacon,
Dmitry Baryshkov, Bjorn Andersson
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel,
Douglas Anderson, Abhinav Kumar, Neil Armstrong,
Krishna Kurapati PSSNV
In-Reply-To: <20240326-rb3gen2-dp-connector-v2-0-a9f1bc32ecaf@quicinc.com>
On Tue, 26 Mar 2024 19:04:17 -0700, Bjorn Andersson wrote:
> RB3Gen2 is capable of producing DisplayPort output on a dedicated
> mini-DP connector and USB Type-C.
>
> Utilize Abel's work for DP vs eDP selection to allow configuring both
> controllers in DP-mode, then enable the two output paths.
>
> Tested by driving fbcon to 4k@60 + 4k@30 concurrently.
>
> [...]
Applied, thanks!
[6/6] arm64: defconfig: Enable sc7280 display and gpu clock controllers
commit: a97b6c42a7b823c429fac562a02d291b47b98d7e
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: qcom: qcs6490-rb3gen2: Enable UFS
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240327-rb3gen2-ufs-v2-1-3de6b5dd78dd@quicinc.com>
On Wed, 27 Mar 2024 19:01:13 -0700, Bjorn Andersson wrote:
> The rb3gen2 has UFS memory, adjust the necessary supply voltage and add
> the controller and phy nodes to enable this.
>
>
Applied, thanks!
[1/1] arm64: dts: qcom: qcs6490-rb3gen2: Enable UFS
commit: 58dc9622d5de6ce0b80969b136e8e09a7645eca5
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: qcom: msm8916-samsung-fortuna: Add touchscreen
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: linux-kernel, Raymond Hackley
Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Stephan Gerhold, Nikita Travkin, linux-arm-msm, devicetree,
~postmarketos/upstreaming, Joe Mason
In-Reply-To: <20240312074536.62964-1-raymondhackley@protonmail.com>
On Tue, 12 Mar 2024 07:45:42 +0000, Raymond Hackley wrote:
> Like msm8916-samsung-a3u-eur, the Grand Prime uses a Zinitix BT541
> touchscreen. Add it together with the necessary fixed-regulator to the
> device tree.
>
>
Applied, thanks!
[1/1] arm64: dts: qcom: msm8916-samsung-fortuna: Add touchscreen
commit: f8dddefcb90eaa339c77b2cb3f5a87dec8b1e3b5
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 0/2] arm64: dts: qcom: msm8916-samsung-fortuna: Add touchscreen and PWM backlight
From: Bjorn Andersson @ 2024-04-04 21:23 UTC (permalink / raw)
To: linux-kernel, Raymond Hackley
Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Stephan Gerhold, Nikita Travkin, linux-arm-msm, devicetree,
~postmarketos/upstreaming
In-Reply-To: <20240404121703.17086-1-raymondhackley@protonmail.com>
On Thu, 04 Apr 2024 12:17:28 +0000, Raymond Hackley wrote:
> Like msm8916-samsung-a3u-eur, the Grand Prime uses a Zinitix BT541
> touchscreen. Add it together with the necessary fixed-regulator to the
> device tree.
>
> Most of the Galaxy Grand Prime use backlight drivers controlled with PWM
> signal.
> To simplify the description, add the backlight with the necessary clk-pwm
> to the common dtsi.
>
> [...]
Applied, thanks!
[1/2] arm64: dts: qcom: msm8916-samsung-fortuna: Add touchscreen
commit: f8dddefcb90eaa339c77b2cb3f5a87dec8b1e3b5
[2/2] arm64: dts: qcom: msm8916-samsung-fortuna: Add PWM backlight
commit: 05c65922bd58cc3fc057b37628b143f76e524496
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH v3] clk: starfive: pll: Fix lower rate of CPUfreq by setting PLL0 rate to 1.5GHz
From: Samuel Holland @ 2024-04-04 21:27 UTC (permalink / raw)
To: Xingyu Wu
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Hal Feng,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
Krzysztof Kozlowski, Michael Turquette, Stephen Boyd,
Conor Dooley, Emil Renner Berthing, Rob Herring,
Krzysztof Kozlowski
In-Reply-To: <NTZPR01MB09566AB865A0266332268DC69F3DA@NTZPR01MB0956.CHNPR01.prod.partner.outlook.cn>
Hi Xingyu,
On 2024-04-03 2:44 AM, Xingyu Wu wrote:
> On 03/04/2024 15:24, Krzysztof Kozlowski wrote:
>>
>> On 03/04/2024 09:19, Xingyu Wu wrote:
>>> On 03/04/2024 0:18, Krzysztof Kozlowski wrote:
>>>>
>>>> On 02/04/2024 11:09, Xingyu Wu wrote:
>>>>> CPUfreq supports 4 cpu frequency loads on 375/500/750/1500MHz.
>>>>> But now PLL0 rate is 1GHz and the cpu frequency loads become
>>>>> 333/500/500/1000MHz in fact.
>>>>>
>>>>> So PLL0 rate should be default set to 1.5GHz. But setting the
>>>>> PLL0 rate need certain steps:
>>>>>
>>>>> 1. Change the parent of cpu_root clock to OSC clock.
>>>>> 2. Change the divider of cpu_core if PLL0 rate is higher than
>>>>> 1.25GHz before CPUfreq boot.
>>>>> 3. Change the parent of cpu_root clock back to PLL0 clock.
>>>>>
>>>>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>>>>> Fixes: e2c510d6d630 ("riscv: dts: starfive: Add cpu scaling for
>>>>> JH7110
>>>>> SoC")
>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>> ---
>>>>>
>>>>> Hi Stephen and Emil,
>>>>>
>>>>> This patch fixes the issue about lower rate of CPUfreq[1] by setting
>>>>> PLL0 rate to 1.5GHz.
>>>>>
>>>>> In order not to affect the cpu operation, setting the PLL0 rate need
>>>>> certain steps. The cpu_root's parent clock should be changed first.
>>>>> And the divider of the cpu_core clock should be set to 2 so they
>>>>> won't crash when setting 1.5GHz without voltage regulation. Due to
>>>>> PLL driver boot earlier than SYSCRG driver, cpu_core and cpu_root
>>>>> clocks are using by ioremap().
>>>>>
>>>>> [1]: https://github.com/starfive-tech/VisionFive2/issues/55
>>>>>
>>>>> Previous patch link:
>>>>> v2:
>>>>> https://lore.kernel.org/all/20230821152915.208366-1-xingyu.wu@starfi
>>>>> ve
>>>>> tech.com/
>>>>> v1:
>>>>> https://lore.kernel.org/all/20230811033631.160912-1-xingyu.wu@starfi
>>>>> ve
>>>>> tech.com/
>>>>>
>>>>> Thanks,
>>>>> Xingyu Wu
>>>>> ---
>>>>> .../jh7110-starfive-visionfive-2.dtsi | 5 +
>>>>> .../clk/starfive/clk-starfive-jh7110-pll.c | 102 ++++++++++++++++++
>>>>
>>>> Please do not mix DTS and driver code. That's not really portable.
>>>> DTS is being exported and used in other projects.
>>>
>>> OK, I will submit that in two patches.
>>>
>>>>
>>>> ...
>>>>
>>>>>
>>>>> @@ -458,6 +535,8 @@ static int jh7110_pll_probe(struct
>>>>> platform_device
>>>> *pdev)
>>>>> struct jh7110_pll_priv *priv;
>>>>> unsigned int idx;
>>>>> int ret;
>>>>> + struct device_node *np;
>>>>> + struct resource res;
>>>>>
>>>>> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>>>> if (!priv)
>>>>> @@ -489,6 +568,29 @@ static int jh7110_pll_probe(struct
>>>>> platform_device
>>>> *pdev)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> + priv->is_first_set = true;
>>>>> + np = of_find_compatible_node(NULL, NULL,
>>>>> +"starfive,jh7110-syscrg");
>>>>
>>>> Your drivers should not do it. It's fragile, hides true link/dependency.
>>>> Please use phandles.
>>>>
>>>>
>>>>> + if (!np) {
>>>>> + ret = PTR_ERR(np);
>>>>> + dev_err(priv->dev, "failed to get syscrg node\n");
>>>>> + goto np_put;
>>>>> + }
>>>>> +
>>>>> + ret = of_address_to_resource(np, 0, &res);
>>>>> + if (ret) {
>>>>> + dev_err(priv->dev, "failed to get syscrg resource\n");
>>>>> + goto np_put;
>>>>> + }
>>>>> +
>>>>> + priv->syscrg_base = ioremap(res.start, resource_size(&res));
>>>>> + if (!priv->syscrg_base)
>>>>> + ret = -ENOMEM;
>>>>
>>>> Why are you mapping other device's IO? How are you going to ensure
>>>> synced access to registers?
>>>
>>> Because setting PLL0 rate need specific steps and use the clocks of SYSCRG.
>>
>> That's not a reason to map other device's IO. That could be a reason for having
>> syscon or some other sort of relationship, like clock or reset.
>>
>>> But SYSCRG driver also need PLL clock to be clock source when adding
>>> clock providers. I tried to add SYSCRG clocks in 'clocks' property in
>>> DT and use
>>> clk_get() to get the clocks. But it could not run and crash. So I use
>>> ioremap() instead.
>>
>> So instead of properly model the relationship, you entangle the drivers even
>> more.
>>
>> Please come with a proper design for this. I have no clue about your hardware,
>> but that looks like you are asynchronously configuring the same hardware in two
>> different places.
>>
>> Sorry, that's poor code.
>>
>> Best regards,
>> Krzysztof
>
> Hi Krzysztof,
>
> If I use the old patch[1] like v2 and set the PLL0 default rate in the SYSCRG driver,
> will it be better?
>
> [1]: https://lore.kernel.org/all/20230821152915.208366-1-xingyu.wu@starfivetech.com/
Both reparenting cpu_root and enforcing the maximum cpu_core frequency can be
accomplished with clk notifiers. See for example ccu_mux_notifier_register() in
drivers/clk/sunxi-ng/ccu_mux.c.
Regards,
Samuel
^ permalink raw reply
* Re: [PATCH v3 09/29] mm: abstract shadow stack vma behind `vma_is_shadow_stack`
From: Deepak Gupta @ 2024-04-04 21:39 UTC (permalink / raw)
To: David Hildenbrand
Cc: paul.walmsley, rick.p.edgecombe, broonie, Szabolcs.Nagy,
kito.cheng, keescook, ajones, conor.dooley, cleger, atishp, alex,
bjorn, alexghiti, samuel.holland, conor, linux-doc, linux-riscv,
linux-kernel, devicetree, linux-mm, linux-arch, linux-kselftest,
corbet, palmer, aou, robh+dt, krzysztof.kozlowski+dt, oleg, akpm,
arnd, ebiederm, Liam.Howlett, vbabka, lstoakes, shuah, brauner,
andy.chiu, jerry.shih, hankuan.chen, greentime.hu, evan,
xiao.w.wang, charlie, apatel, mchitale, dbarboza, sameo,
shikemeng, willy, vincent.chen, guoren, samitolvanen,
songshuaishuai, gerg, heiko, bhe, jeeheng.sia, cyy, maskray,
ancientmodern4, mathis.salmen, cuiyunhui, bgray, mpe, baruch, alx,
catalin.marinas, revest, josh, shr, deller, omosnace, ojeda,
jhubbard, Mike Rapoport
In-Reply-To: <c438ea3a-24bc-470b-a2eb-6e7517bd4362@redhat.com>
On Thu, Apr 04, 2024 at 09:02:17PM +0200, David Hildenbrand wrote:
>On 04.04.24 01:34, Deepak Gupta wrote:
>> }
>>- } else if (!(vm_flags & VM_READ)) {
>>+ } else if (!(vm_flags & VM_READ) && !vma_is_shadow_stack(vm_flags)) {
>>+ /* reads allowed if its shadow stack vma */
>> if (!(gup_flags & FOLL_FORCE))
>> return -EFAULT;
>> /*
>
>Unless I am missing something, this is not a simple cleanup. It should
>go into a separate patch with a clearly documented reason for that
>change.
I tried that here
https://lore.kernel.org/linux-mm/CAKC1njTPBqtsAOn-CWhB+-8FaZ2KWkkz-vRZr7MZq=0yLUdjcQ@mail.gmail.com/T/
But at that time, VM_SHADOW_STACK for riscv meant only VM_WRITE. So I think
there was obvious uneasiness with that part.
Now we have VM_SHADOW_STACK pretty much same for all arches and only 64bit.
I'll try it again as a separate patch.
>
>--
>Cheers,
>
>David / dhildenb
>
^ permalink raw reply
* [PATCH v2 0/2] Input: add ft5426
From: Andreas Kemnade @ 2024-04-04 22:20 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
andreas, o.rempel, u.kleine-koenig, hdegoede, andy.shevchenko,
ye.xingchen, p.puschmann, linux-input, devicetree, linux-kernel,
caleb.connolly
Add ft5426 touchscreen controller and the corresponding compatible.
Changes in v2:
- reorder compatible
Andreas Kemnade (2):
dt-bindings: input: touchscreen: edt-ft5x06: Add ft5426
Input: edt-ft5x06 - add ft5426
.../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 +
drivers/input/touchscreen/edt-ft5x06.c | 1 +
2 files changed, 2 insertions(+)
--
2.39.2
^ permalink raw reply
* [PATCH v2 2/2] Input: edt-ft5x06 - add ft5426
From: Andreas Kemnade @ 2024-04-04 22:20 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
andreas, o.rempel, u.kleine-koenig, hdegoede, andy.shevchenko,
ye.xingchen, p.puschmann, linux-input, devicetree, linux-kernel,
caleb.connolly
In-Reply-To: <20240404222009.670685-1-andreas@kemnade.info>
As ft5426 seems to be compatible with this driver, add it.
Debug output during identification: Model "generic ft5x06 (79)", Rev. "
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
drivers/input/touchscreen/edt-ft5x06.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 2a1db1134476..4e7621a80175 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1484,6 +1484,7 @@ static const struct of_device_id edt_ft5x06_of_match[] = {
{ .compatible = "edt,edt-ft5206", .data = &edt_ft5x06_data },
{ .compatible = "edt,edt-ft5306", .data = &edt_ft5x06_data },
{ .compatible = "edt,edt-ft5406", .data = &edt_ft5x06_data },
+ { .compatible = "focaltech,ft5426", .data = &edt_ft5506_data },
{ .compatible = "edt,edt-ft5506", .data = &edt_ft5506_data },
{ .compatible = "evervision,ev-ft5726", .data = &edt_ft5506_data },
/* Note focaltech vendor prefix for compatibility with ft6236.c */
--
2.39.2
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: input: touchscreen: edt-ft5x06: Add ft5426
From: Andreas Kemnade @ 2024-04-04 22:20 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
andreas, o.rempel, u.kleine-koenig, hdegoede, andy.shevchenko,
ye.xingchen, p.puschmann, linux-input, devicetree, linux-kernel,
caleb.connolly
Cc: Krzysztof Kozlowski
In-Reply-To: <20240404222009.670685-1-andreas@kemnade.info>
Add compatible for ft5426.
Searches for documentation reveal neither edt nor evervision
as some related company, only FocalTech.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
index f2808cb4d99d..71fd3c66c966 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
@@ -39,6 +39,7 @@ properties:
- edt,edt-ft5406
- edt,edt-ft5506
- evervision,ev-ft5726
+ - focaltech,ft5426
- focaltech,ft6236
reg:
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v3 12/25] media: i2c: imx258: Allow configuration of clock lane behaviour
From: Luigi311 @ 2024-04-04 22:29 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, phone-devel
In-Reply-To: <Zg2kcI/1Gdgt0ilB@duo.ucw.cz>
On 4/3/24 12:48, Pavel Machek wrote:
> Hi!
>
>> The sensor supports the clock lane either remaining in HS mode
>> during frame blanking, or dropping to LP11.
>>
>> Add configuration of the mode via V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK.
>
>> + ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP,
>> + IMX258_REG_VALUE_08BIT,
>> + imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ?
>> + 1 : 0);
>
> !! can be used to turn value into 1/0. I find it easier to read than ?
> 1 : 0 combination, but possibly that's fine, too.
>
> Best regards,
> Pavel
>
I assume you mean by using
!!(imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)
I can go ahead and use that instead
^ permalink raw reply
* Re: [PATCH v3 19/25] media: i2c: imx258: Change register settings for variants of the sensor
From: Luigi311 @ 2024-04-04 22:44 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel
In-Reply-To: <Zg2BZXQpzsm7jMnc@kekkonen.localdomain>
On 4/3/24 10:18, Sakari Ailus wrote:
> Hi Luis, Dave,
>
> On Wed, Apr 03, 2024 at 09:03:48AM -0600, git@luigi311.com wrote:
>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>
>> Sony have advised that there are variants of the IMX258 sensor which
>> require slightly different register configuration to the mainline
>> imx258 driver defaults.
>>
>> There is no available run-time detection for the variant, so add
>> configuration via the DT compatible string.
>>
>> The Vision Components imx258 module supports PDAF, so add the
>> register differences for that variant
>>
>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>> Signed-off-by: Luis Garcia <git@luigi311.com>
>> ---
>> drivers/media/i2c/imx258.c | 48 ++++++++++++++++++++++++++++++++++----
>> 1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>> index 775d957c9b87..fa48da212037 100644
>> --- a/drivers/media/i2c/imx258.c
>> +++ b/drivers/media/i2c/imx258.c
>> @@ -6,6 +6,7 @@
>> #include <linux/delay.h>
>> #include <linux/i2c.h>
>> #include <linux/module.h>
>> +#include <linux/of_device.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/regulator/consumer.h>
>> #include <media/v4l2-ctrls.h>
>> @@ -321,8 +322,6 @@ static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
>>
>> static const struct imx258_reg mode_common_regs[] = {
>> { 0x3051, 0x00 },
>> - { 0x3052, 0x00 },
>> - { 0x4E21, 0x14 },
>> { 0x6B11, 0xCF },
>> { 0x7FF0, 0x08 },
>> { 0x7FF1, 0x0F },
>> @@ -345,7 +344,6 @@ static const struct imx258_reg mode_common_regs[] = {
>> { 0x7FA8, 0x03 },
>> { 0x7FA9, 0xFE },
>> { 0x7B24, 0x81 },
>> - { 0x7B25, 0x00 },
>> { 0x6564, 0x07 },
>> { 0x6B0D, 0x41 },
>> { 0x653D, 0x04 },
>> @@ -460,6 +458,33 @@ static const struct imx258_reg mode_1048_780_regs[] = {
>> { 0x034F, 0x0C },
>> };
>>
>> +struct imx258_variant_cfg {
>> + const struct imx258_reg *regs;
>> + unsigned int num_regs;
>> +};
>> +
>> +static const struct imx258_reg imx258_cfg_regs[] = {
>> + { 0x3052, 0x00 },
>> + { 0x4E21, 0x14 },
>> + { 0x7B25, 0x00 },
>> +};
>> +
>> +static const struct imx258_variant_cfg imx258_cfg = {
>> + .regs = imx258_cfg_regs,
>> + .num_regs = ARRAY_SIZE(imx258_cfg_regs),
>> +};
>> +
>> +static const struct imx258_reg imx258_pdaf_cfg_regs[] = {
>> + { 0x3052, 0x01 },
>> + { 0x4E21, 0x10 },
>> + { 0x7B25, 0x01 },
>> +};
>> +
>> +static const struct imx258_variant_cfg imx258_pdaf_cfg = {
>> + .regs = imx258_pdaf_cfg_regs,
>> + .num_regs = ARRAY_SIZE(imx258_pdaf_cfg_regs),
>> +};
>> +
>> static const char * const imx258_test_pattern_menu[] = {
>> "Disabled",
>> "Solid Colour",
>> @@ -637,6 +662,8 @@ struct imx258 {
>> struct v4l2_subdev sd;
>> struct media_pad pad;
>>
>> + const struct imx258_variant_cfg *variant_cfg;
>> +
>> struct v4l2_ctrl_handler ctrl_handler;
>> /* V4L2 Controls */
>> struct v4l2_ctrl *link_freq;
>> @@ -1104,6 +1131,14 @@ static int imx258_start_streaming(struct imx258 *imx258)
>> return ret;
>> }
>>
>> + ret = imx258_write_regs(imx258, imx258->variant_cfg->regs,
>> + imx258->variant_cfg->num_regs);
>> + if (ret) {
>> + dev_err(&client->dev, "%s failed to set variant config\n",
>> + __func__);
>> + return ret;
>> + }
>> +
>> ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP,
>> IMX258_REG_VALUE_08BIT,
>> imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ?
>> @@ -1492,6 +1527,10 @@ static int imx258_probe(struct i2c_client *client)
>>
>> imx258->csi2_flags = ep.bus.mipi_csi2.flags;
>>
>> + imx258->variant_cfg = of_device_get_match_data(&client->dev);
>
> You'll also need to keep this working for ACPI based systems. I.e. in
> practice remove "of_" prefix here and add the non-PDAF variant data to the
> relevant ACPI ID list.
>
Removing of_ is easy enough and looking at all the other commits that make
this change in other drivers I dont see anything else being done besides
adding in the .data section that is down below for both imx258 and pdaf
versions. Is that what you are referencing or is there some other place
to add variant data to ACPI ID list?
>> + if (!imx258->variant_cfg)
>> + imx258->variant_cfg = &imx258_cfg;
>> +
>> /* Initialize subdev */
>> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>>
>> @@ -1579,7 +1618,8 @@ MODULE_DEVICE_TABLE(acpi, imx258_acpi_ids);
>> #endif
>>
>> static const struct of_device_id imx258_dt_ids[] = {
>> - { .compatible = "sony,imx258" },
>> + { .compatible = "sony,imx258", .data = &imx258_cfg },
>> + { .compatible = "sony,imx258-pdaf", .data = &imx258_pdaf_cfg },
>> { /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(of, imx258_dt_ids);
>
^ permalink raw reply
* Re: [PATCH 08/17] clk: samsung: gs101: add support for cmu_hsi2
From: kernel test robot @ 2024-04-04 22:52 UTC (permalink / raw)
To: Peter Griffin, mturquette, sboyd, robh, krzk+dt, conor+dt, vkoul,
kishon, alim.akhtar, avri.altman, bvanassche, s.nawrocki,
cw00.choi, jejb, martin.petersen, chanho61.park, ebiggers
Cc: oe-kbuild-all, linux-scsi, linux-phy, devicetree, linux-clk,
linux-samsung-soc, linux-kernel, linux-arm-kernel, tudor.ambarus,
andre.draszik, saravanak, willmcvicker, Peter Griffin
In-Reply-To: <20240404122559.898930-9-peter.griffin@linaro.org>
Hi Peter,
kernel test robot noticed the following build warnings:
[auto build test WARNING on krzk/for-next]
[also build test WARNING on robh/for-next clk/clk-next linus/master v6.9-rc2 next-20240404]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Peter-Griffin/dt-bindings-clock-google-gs101-clock-add-HSI2-clock-management-unit/20240404-205113
base: https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git for-next
patch link: https://lore.kernel.org/r/20240404122559.898930-9-peter.griffin%40linaro.org
patch subject: [PATCH 08/17] clk: samsung: gs101: add support for cmu_hsi2
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240405/202404050633.EZfOttFD-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240405/202404050633.EZfOttFD-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404050633.EZfOttFD-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/clk/samsung/clk-gs101.c:16:
>> drivers/clk/samsung/clk-gs101.c:3640:7: warning: 'mout_hsi2_mmc_card_p' defined but not used [-Wunused-const-variable=]
3640 | PNAME(mout_hsi2_mmc_card_p) = { "fout_shared2_pll", "fout_shared3_pll",
| ^~~~~~~~~~~~~~~~~~~~
drivers/clk/samsung/clk.h:229:44: note: in definition of macro 'PNAME'
229 | #define PNAME(x) static const char * const x[] __initconst
| ^
>> drivers/clk/samsung/clk-gs101.c:3633:7: warning: 'mout_hsi2_bus_p' defined but not used [-Wunused-const-variable=]
3633 | PNAME(mout_hsi2_bus_p) = { "dout_cmu_shared0_div4",
| ^~~~~~~~~~~~~~~
drivers/clk/samsung/clk.h:229:44: note: in definition of macro 'PNAME'
229 | #define PNAME(x) static const char * const x[] __initconst
| ^
>> drivers/clk/samsung/clk-gs101.c:3631:7: warning: 'mout_hsi2_pcie_p' defined but not used [-Wunused-const-variable=]
3631 | PNAME(mout_hsi2_pcie_p) = { "oscclk", "dout_cmu_shared2_div2" };
| ^~~~~~~~~~~~~~~~
drivers/clk/samsung/clk.h:229:44: note: in definition of macro 'PNAME'
229 | #define PNAME(x) static const char * const x[] __initconst
| ^
>> drivers/clk/samsung/clk-gs101.c:3628:7: warning: 'mout_hsi2_ufs_embd_p' defined but not used [-Wunused-const-variable=]
3628 | PNAME(mout_hsi2_ufs_embd_p) = { "oscclk", "dout_cmu_shared0_div4",
| ^~~~~~~~~~~~~~~~~~~~
drivers/clk/samsung/clk.h:229:44: note: in definition of macro 'PNAME'
229 | #define PNAME(x) static const char * const x[] __initconst
| ^
vim +/mout_hsi2_mmc_card_p +3640 drivers/clk/samsung/clk-gs101.c
3627
> 3628 PNAME(mout_hsi2_ufs_embd_p) = { "oscclk", "dout_cmu_shared0_div4",
3629 "dout_cmu_shared2_div2", "fout_spare_pll" };
3630
> 3631 PNAME(mout_hsi2_pcie_p) = { "oscclk", "dout_cmu_shared2_div2" };
3632
> 3633 PNAME(mout_hsi2_bus_p) = { "dout_cmu_shared0_div4",
3634 "dout_cmu_shared1_div4",
3635 "dout_cmu_shared2_div2",
3636 "dout_cmu_shared3_div2",
3637 "fout_spare_pll", "oscclk", "oscclk",
3638 "oscclk" };
3639
> 3640 PNAME(mout_hsi2_mmc_card_p) = { "fout_shared2_pll", "fout_shared3_pll",
3641 "dout_cmu_shared0_div4", "fout_spare_pll" };
3642
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add Qualcomm PCIe ECAM root complex driver
From: Mayank Rana @ 2024-04-04 23:02 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-pci, lpieralisi, kw, robh, bhelgaas,
andersson, manivannan.sadhasivam, krzysztof.kozlowski+dt,
conor+dt, devicetree
Cc: linux-arm-msm, quic_ramkri, quic_nkela, quic_shazhuss,
quic_msarkar, quic_nitegupt
In-Reply-To: <42d1281e-9546-4af1-a30b-8a0c3969be6b@linaro.org>
Hi Krzysztof
On 4/4/2024 12:33 PM, Krzysztof Kozlowski wrote:
> On 04/04/2024 21:11, Mayank Rana wrote:
>> On some of Qualcomm platform, firmware takes care of system resources
>> related to PCIe PHY and controller as well bringing up PCIe link and
>> having static iATU configuration for PCIe controller to work into
>> ECAM compliant mode. Hence add Qualcomm PCIe ECAM root complex driver.
>>
>> Tested:
>> - Validated NVME functionality with PCIe0 and PCIe1 on SA877p-ride platform
>>
>
> RFC means code is not ready, right? Please get internal review done and
> send it when it is ready. I am not sure if you expect any reviews. Some
> people send RFC and do not expect reviews. Some expect. I have no clue
> and I do not want to waste my time. Please clarify what you expect from
> maintainers regarding this contribution.
>
> Best regards,
> Krzysztof
>
Thanks for initial comments.
yes, this is work in progress. There are still more functionalities
planned to be added as part of this driver. Although purpose of sending
initial change here to get feedback and review comments in terms of
usage of generic Qualcomm PCIe ECAM driver, and usage of MSI
functionality with it. I missed mentioning this as part of cover letter.
So please help to review and provide feedback.
Regards,
Mayank
^ permalink raw reply
* Re: [PATCH 1/3] of: Add a helper to free property struct
From: Saravana Kannan @ 2024-04-04 23:09 UTC (permalink / raw)
To: Rob Herring; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <20240404-dt-cleanup-free-v1-1-c60e6cba8da9@kernel.org>
On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
>
> Freeing a property struct is 3 kfree()'s which is duplicated in multiple
> spots. Add a helper, __of_prop_free(), and replace all the open coded
> cases in the DT code.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> drivers/of/dynamic.c | 26 ++++++++++++--------------
> drivers/of/of_private.h | 1 +
> drivers/of/overlay.c | 11 +++--------
> drivers/of/unittest.c | 12 +++---------
> 4 files changed, 19 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 3bf27052832f..af7c57a7a25d 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -305,15 +305,20 @@ int of_detach_node(struct device_node *np)
> }
> EXPORT_SYMBOL_GPL(of_detach_node);
>
> +void __of_prop_free(struct property *prop)
> +{
> + kfree(prop->name);
> + kfree(prop->value);
> + kfree(prop);
> +}
> +
> static void property_list_free(struct property *prop_list)
> {
> struct property *prop, *next;
>
> for (prop = prop_list; prop != NULL; prop = next) {
> next = prop->next;
> - kfree(prop->name);
> - kfree(prop->value);
> - kfree(prop);
> + __of_prop_free(prop);
> }
> }
>
> @@ -426,9 +431,7 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
> return new;
>
> err_free:
> - kfree(new->name);
> - kfree(new->value);
> - kfree(new);
> + __of_prop_free(new);
> return NULL;
> }
>
> @@ -470,9 +473,7 @@ struct device_node *__of_node_dup(const struct device_node *np,
> if (!new_pp)
> goto err_prop;
> if (__of_add_property(node, new_pp)) {
> - kfree(new_pp->name);
> - kfree(new_pp->value);
> - kfree(new_pp);
> + __of_prop_free(new_pp);
> goto err_prop;
> }
> }
> @@ -921,11 +922,8 @@ static int of_changeset_add_prop_helper(struct of_changeset *ocs,
> return -ENOMEM;
>
> ret = of_changeset_add_property(ocs, np, new_pp);
> - if (ret) {
> - kfree(new_pp->name);
> - kfree(new_pp->value);
> - kfree(new_pp);
> - }
> + if (ret)
> + __of_prop_free(new_pp);
>
> return ret;
> }
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 485483524b7f..94fc0aa07af9 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -123,6 +123,7 @@ extern void *__unflatten_device_tree(const void *blob,
> * own the devtree lock or work on detached trees only.
> */
> struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags);
> +void __of_prop_free(struct property *prop);
> struct device_node *__of_node_dup(const struct device_node *np,
> const char *full_name);
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 2ae7e9d24a64..4d861a75d694 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -262,9 +262,7 @@ static struct property *dup_and_fixup_symbol_prop(
> return new_prop;
>
> err_free_new_prop:
> - kfree(new_prop->name);
> - kfree(new_prop->value);
> - kfree(new_prop);
> + __of_prop_free(new_prop);
> err_free_target_path:
> kfree(target_path);
>
> @@ -361,11 +359,8 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
> pr_err("WARNING: memory leak will occur if overlay removed, property: %pOF/%s\n",
> target->np, new_prop->name);
>
> - if (ret) {
> - kfree(new_prop->name);
> - kfree(new_prop->value);
> - kfree(new_prop);
> - }
> + if (ret)
> + __of_prop_free(new_prop);
> return ret;
> }
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 6b5c36b6a758..a8c01c953a29 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -795,15 +795,11 @@ static void __init of_unittest_property_copy(void)
>
> new = __of_prop_dup(&p1, GFP_KERNEL);
> unittest(new && propcmp(&p1, new), "empty property didn't copy correctly\n");
> - kfree(new->value);
> - kfree(new->name);
> - kfree(new);
> + __of_prop_free(new);
>
> new = __of_prop_dup(&p2, GFP_KERNEL);
> unittest(new && propcmp(&p2, new), "non-empty property didn't copy correctly\n");
> - kfree(new->value);
> - kfree(new->name);
> - kfree(new);
> + __of_prop_free(new);
> #endif
> }
>
> @@ -3718,9 +3714,7 @@ static __init void of_unittest_overlay_high_level(void)
> goto err_unlock;
> }
> if (__of_add_property(of_symbols, new_prop)) {
> - kfree(new_prop->name);
> - kfree(new_prop->value);
> - kfree(new_prop);
> + __of_prop_free(new_prop);
> /* "name" auto-generated by unflatten */
> if (!strcmp(prop->name, "name"))
> continue;
>
Reviewed-by: Saravana Kannan <saravanak@google.com>
-Saravana
^ permalink raw reply
* Re: [PATCH 2/3] of: Use scope based kfree() cleanups
From: Saravana Kannan @ 2024-04-04 23:15 UTC (permalink / raw)
To: Rob Herring; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <20240404-dt-cleanup-free-v1-2-c60e6cba8da9@kernel.org>
On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
>
> Use the relatively new scope based kfree() cleanup to simplify error
> handling. Doing so reduces the chances of memory leaks and simplifies
> error paths by avoiding the need for goto statements.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> drivers/of/base.c | 34 ++++++++--------------------------
> drivers/of/dynamic.c | 11 ++++-------
> drivers/of/resolver.c | 35 +++++++++++++----------------------
> 3 files changed, 25 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 8856c67c466a..20603d3c9931 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -16,6 +16,7 @@
>
> #define pr_fmt(fmt) "OF: " fmt
>
> +#include <linux/cleanup.h>
> #include <linux/console.h>
> #include <linux/ctype.h>
> #include <linux/cpu.h>
> @@ -1393,8 +1394,10 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> const char *stem_name,
> int index, struct of_phandle_args *out_args)
> {
> - char *cells_name, *map_name = NULL, *mask_name = NULL;
> - char *pass_name = NULL;
> + char *cells_name __free(kfree) = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
> + char *map_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map", stem_name);
> + char *mask_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-mask", stem_name);
> + char *pass_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-pass-thru", stem_name);
With the scoped stuff, do these function calls need to be in the same
line we are defining these variables? If not, I'd rather that the
calls remain where they were. It feels like a lote to visually parse
and take in from a readability perspective.
> struct device_node *cur, *new = NULL;
> const __be32 *map, *mask, *pass;
> static const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = cpu_to_be32(~0) };
> @@ -1407,27 +1410,13 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> if (index < 0)
> return -EINVAL;
>
> - cells_name = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
> - if (!cells_name)
> + if (!cells_name || !map_name || !mask_name || !pass_name)
> return -ENOMEM;
>
> - ret = -ENOMEM;
> - map_name = kasprintf(GFP_KERNEL, "%s-map", stem_name);
> - if (!map_name)
> - goto free;
> -
> - mask_name = kasprintf(GFP_KERNEL, "%s-map-mask", stem_name);
> - if (!mask_name)
> - goto free;
> -
> - pass_name = kasprintf(GFP_KERNEL, "%s-map-pass-thru", stem_name);
> - if (!pass_name)
> - goto free;
> -
> ret = __of_parse_phandle_with_args(np, list_name, cells_name, -1, index,
> out_args);
> if (ret)
> - goto free;
> + return ret;
>
> /* Get the #<list>-cells property */
> cur = out_args->np;
> @@ -1444,8 +1433,7 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> /* Get the <list>-map property */
> map = of_get_property(cur, map_name, &map_len);
> if (!map) {
> - ret = 0;
> - goto free;
> + return 0;
> }
> map_len /= sizeof(u32);
>
> @@ -1521,12 +1509,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> put:
> of_node_put(cur);
> of_node_put(new);
> -free:
> - kfree(mask_name);
> - kfree(map_name);
> - kfree(cells_name);
> - kfree(pass_name);
> -
> return ret;
> }
> EXPORT_SYMBOL(of_parse_phandle_with_args_map);
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index af7c57a7a25d..43f4e2c93bd2 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -9,6 +9,7 @@
>
> #define pr_fmt(fmt) "OF: " fmt
>
> +#include <linux/cleanup.h>
> #include <linux/of.h>
> #include <linux/spinlock.h>
> #include <linux/slab.h>
> @@ -1019,10 +1020,9 @@ int of_changeset_add_prop_u32_array(struct of_changeset *ocs,
> const u32 *array, size_t sz)
> {
> struct property prop;
> - __be32 *val;
> - int i, ret;
> + __be32 *val __free(kfree) = kcalloc(sz, sizeof(__be32), GFP_KERNEL);
> + int i;
>
> - val = kcalloc(sz, sizeof(__be32), GFP_KERNEL);
> if (!val)
> return -ENOMEM;
>
> @@ -1032,9 +1032,6 @@ int of_changeset_add_prop_u32_array(struct of_changeset *ocs,
> prop.length = sizeof(u32) * sz;
> prop.value = (void *)val;
>
> - ret = of_changeset_add_prop_helper(ocs, np, &prop);
> - kfree(val);
> -
> - return ret;
> + return of_changeset_add_prop_helper(ocs, np, &prop);
> }
> EXPORT_SYMBOL_GPL(of_changeset_add_prop_u32_array);
> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
> index b278ab4338ce..2780928764a4 100644
> --- a/drivers/of/resolver.c
> +++ b/drivers/of/resolver.c
> @@ -8,6 +8,7 @@
>
> #define pr_fmt(fmt) "OF: resolver: " fmt
>
> +#include <linux/cleanup.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/of.h>
> @@ -74,11 +75,11 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
> {
> struct device_node *refnode;
> struct property *prop;
> - char *value, *cur, *end, *node_path, *prop_name, *s;
> + char *value __free(kfree) = kmemdup(prop_fixup->value, prop_fixup->length, GFP_KERNEL);
> + char *cur, *end, *node_path, *prop_name, *s;
> int offset, len;
> int err = 0;
>
> - value = kmemdup(prop_fixup->value, prop_fixup->length, GFP_KERNEL);
> if (!value)
> return -ENOMEM;
>
> @@ -89,23 +90,19 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
>
> node_path = cur;
> s = strchr(cur, ':');
> - if (!s) {
> - err = -EINVAL;
> - goto err_fail;
> - }
> + if (!s)
> + return -EINVAL;
> *s++ = '\0';
>
> prop_name = s;
> s = strchr(s, ':');
> - if (!s) {
> - err = -EINVAL;
> - goto err_fail;
> - }
> + if (!s)
> + return -EINVAL;
> *s++ = '\0';
>
> err = kstrtoint(s, 10, &offset);
> if (err)
> - goto err_fail;
> + return err;
>
> refnode = __of_find_node_by_full_path(of_node_get(overlay), node_path);
> if (!refnode)
> @@ -117,22 +114,16 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
> }
> of_node_put(refnode);
>
> - if (!prop) {
> - err = -ENOENT;
> - goto err_fail;
> - }
> + if (!prop)
> + return -ENOENT;
>
> - if (offset < 0 || offset + sizeof(__be32) > prop->length) {
> - err = -EINVAL;
> - goto err_fail;
> - }
> + if (offset < 0 || offset + sizeof(__be32) > prop->length)
> + return -EINVAL;
>
> *(__be32 *)(prop->value + offset) = cpu_to_be32(phandle);
> }
>
> -err_fail:
> - kfree(value);
> - return err;
> + return 0;
> }
>
> /* compare nodes taking into account that 'name' strips out the @ part */
Reviewed-by: Saravana Kannan <saravanak@google.com>
-Saravana
^ permalink raw reply
* Re: [PATCH v3 23/25] drivers: media: i2c: imx258: Add support for powerdown gpio
From: Luis Garcia @ 2024-04-04 23:18 UTC (permalink / raw)
To: Dave Stevenson
Cc: Ondřej Jirman, Sakari Ailus, linux-media, jacopo.mondi,
mchehab, robh, krzysztof.kozlowski+dt, conor+dt, shawnguo,
s.hauer, kernel, festevam, devicetree, imx, linux-arm-kernel,
linux-kernel, pavel, phone-devel
In-Reply-To: <CAPY8ntAcB3wyLj1wNE5YBx0_UGRiXEv6057XfEBfjk8NOLC9yQ@mail.gmail.com>
On 4/4/24 08:12, Dave Stevenson wrote:
> Hi Luigi
>
> On Wed, 3 Apr 2024 at 20:34, Luigi311 <git@luigi311.com> wrote:
>>
>> On 4/3/24 10:57, Ondřej Jirman wrote:
>>> Hi Sakari and Luis,
>>>
>>> On Wed, Apr 03, 2024 at 04:25:41PM GMT, Sakari Ailus wrote:
>>>> Hi Luis, Ondrej,
>>>>
>>>> On Wed, Apr 03, 2024 at 09:03:52AM -0600, git@luigi311.com wrote:
>>>>> From: Luis Garcia <git@luigi311.com>
>>>>>
>>>>> On some boards powerdown signal needs to be deasserted for this
>>>>> sensor to be enabled.
>>>>>
>>>>> Signed-off-by: Ondrej Jirman <megi@xff.cz>
>>>>> Signed-off-by: Luis Garcia <git@luigi311.com>
>>>>> ---
>>>>> drivers/media/i2c/imx258.c | 13 +++++++++++++
>>>>> 1 file changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>>>>> index 30352c33f63c..163f04f6f954 100644
>>>>> --- a/drivers/media/i2c/imx258.c
>>>>> +++ b/drivers/media/i2c/imx258.c
>>>>> @@ -679,6 +679,8 @@ struct imx258 {
>>>>> unsigned int lane_mode_idx;
>>>>> unsigned int csi2_flags;
>>>>>
>>>>> + struct gpio_desc *powerdown_gpio;
>>>>> +
>>>>> /*
>>>>> * Mutex for serialized access:
>>>>> * Protect sensor module set pad format and start/stop streaming safely.
>>>>> @@ -1213,6 +1215,8 @@ static int imx258_power_on(struct device *dev)
>>>>> struct imx258 *imx258 = to_imx258(sd);
>>>>> int ret;
>>>>>
>>>>> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 0);
>>>>
>>>> What does the spec say? Should this really happen before switching on the
>>>> supplies below?
>>>
>>> There's no powerdown input in the IMX258 manual. The manual only mentions
>>> that XCLR (reset) should be held low during power on.
>>>
>>> https://megous.com/dl/tmp/15b0992a720ab82d.png
>>>
>>> https://megous.com/dl/tmp/f2cc991046d97641.png
>>>
>>> This sensor doesn’t have a built-in “Power ON Reset” function. The XCLR pin
>>> is set to “LOW” and the power supplies are brought up. Then the XCLR pin
>>> should be set to “High” after INCK supplied.
>>>
>>> So this input is some feature on camera module itself outside of the
>>> IMX258 chip, which I think is used to gate power to the module. Eg. on Pinephone
>>> Pro, there are two modules with shared power rails, so enabling supply to
>>> one module enables it to the other one, too. So this input becomes the only way
>>> to really enable/disable power to the chip when both are used at once at some
>>> point, because regulator_bulk_enable/disable becomes ineffective at that point.
>>>
>>> Luis, maybe you saw some other datasheet that mentions this input? IMO,
>>> it just gates the power rails via some mosfets on the module itself, since
>>> there's not power down input to the chip itself.
>>>
>>> kind regards,
>>> o.
>>>
>>
>> Ondrej, I did not see anything else in the datasheet since I'm pretty sure
>> I'm looking at the same datasheet as it was supplied to me by Pine64. I'm
>> not sure what datasheet Dave has access to since he got his for a
>> completely different module than what we are testing with though.
>
> I only have a leaked datasheet (isn't the internet wonderful!) [1]
> XCLR is documented in that, as Ondrej has said.
>
> If this powerdown GPIO is meant to be driving XCLR, then it is in the
> wrong order against the supplies.
>
> This does make me confused over the difference between this powerdown
> GPIO and the reset GPIO that you implement in 24/25.
>
> Following the PinePhone Pro DT [3] and schematics [4]
> reset-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_LOW>;
> powerdown-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
>
> Schematic page 11 upper right block
> GPIO1_A0/ISP0_SHUTTER_EN/ISP1_SHUTTER_EN/TCPD_VBUS_SINK_EN_d becomes
> Camera_RST_L. Page 18 feeds that through to the RESET on the camera
> connector.
> Page 11 left middle block GPIO2_D4/SDIO0_BKPWR_d becomes DVP_PDN1_H.
> Page 18 feeds that through to the PWDN on the camera connector.
>
> Seeing as we apparently have a lens driver kicking around as well,
> potentially one is reset to the VCM, and one to the sensor? DW9714
> does have an XSD shutdown pin.
> Only the module integrator is going to really know the answer,
> although potentially a little poking with gpioset and i2cdetect may
> tell you more.
>
> Dave
>
> [1] https://web.archive.org/web/20201027131326/www.hi.app/IMX258-datasheet.pdf
> [2] https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
> [3] https://xff.cz/git/linux/tree/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts?h=orange-pi-5.18#n868
> [4] https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
>
>
Out of curiosity I dropped this and tested it on my PPP and it still loads
up the camera correctly so I am fine with dropping this and patch 22 that
adds in the dt binding
>>>>> +
>>>>> ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
>>>>> imx258->supplies);
>>>>> if (ret) {
>>>>> @@ -1224,6 +1228,7 @@ static int imx258_power_on(struct device *dev)
>>>>> ret = clk_prepare_enable(imx258->clk);
>>>>> if (ret) {
>>>>> dev_err(dev, "failed to enable clock\n");
>>>>> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
>>>>> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>>>>> }
>>>>>
>>>>> @@ -1238,6 +1243,8 @@ static int imx258_power_off(struct device *dev)
>>>>> clk_disable_unprepare(imx258->clk);
>>>>> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>>>>>
>>>>> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
>>>>> +
>>>>> return 0;
>>>>> }
>>>>>
>>>>> @@ -1541,6 +1548,12 @@ static int imx258_probe(struct i2c_client *client)
>>>>> if (!imx258->variant_cfg)
>>>>> imx258->variant_cfg = &imx258_cfg;
>>>>>
>>>>> + /* request optional power down pin */
>>>>> + imx258->powerdown_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
>>>>> + GPIOD_OUT_HIGH);
>>>>> + if (IS_ERR(imx258->powerdown_gpio))
>>>>> + return PTR_ERR(imx258->powerdown_gpio);
>>>>> +
>>>>> /* Initialize subdev */
>>>>> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>>>>>
>>>>
>>>> --
>>>> Regards,
>>>>
>>>> Sakari Ailus
>>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox