Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Dmitry Baryshkov @ 2024-04-05 12:49 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Jeff Johnson, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio, ath10k, linux-wireless, netdev,
	devicetree, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <87sf002d8d.fsf@kernel.org>

On Fri, 5 Apr 2024 at 15:41, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
> >> >>
> >> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >> >>
> >> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> >> >> hand that means we need to update linux-firmware (basically add a new
> >> >> >> symlink) everytime a new product is added. But are there going to be
> >> >> >> that many new ath10k based products?
> >> >> >>
> >> >> >> Using 'qcm2290' is easier because for a new product then there only
> >> >> >> needs to be a change in DTS and no need to change anything
> >> >> >> linux-firmware. But here the risk is that if there's actually two
> >> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> >> >> directory?
> >> >> >>
> >> >> >> But I don't really know, thoughts?
> >> >> >
> >> >> > After some thought, I'd suggest to follow approach taken by the rest
> >> >> > of qcom firmware:
> >> >>
> >> >> Can you provide pointers to those cases?
> >> >
> >> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
> >> >
> >> >>
> >> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
> >> >> > and then put a vendor-specific firmware into subdir. If any of such
> >> >> > vendors appear, we might even implement structural fallback: first
> >> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> >> >> > finally just under hw1.0.
> >> >>
> >> >> Honestly that looks quite compilicated compared to having just one
> >> >> sub-directory. How will ath10k find the directory names (or I vendor and
> >> >> model names) like 'Google' or 'blueline' in this example?
> >> >
> >> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
> >> > this can be really simpler, firmware-name = "blueline" or
> >> > "sdm845/blueline" with no need for fallbacks.
> >>
> >> I have been also thinking about this and I would prefer not to have the
> >> fallbacks. But good if you agree with that.
> >>
> >> IMHO just "sdm845-blueline" would be the most simple. I don't see the
> >> point of having a directory structure when there are not that many
> >> directories really. But this is just cosmetics.
> >
> > It is "not many directories" if we are thinking about the
> > linux-firmware or open devices. But once embedded distros start
> > picking this up for the supported devices, this can quickly become a
> > nuisance.
>
> Ok. Just out of curiosity, any ideas how many devices/products are there
> with wcn3990 who want to use ath10k?

Just for the DT in mainline I can count about 30 devices that have
ath10k WiFi node:

arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-xiaomi-sagit.dts:&wifi {
arch/arm64/boot/dts/qcom/qcs404-evb.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-idp.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-lenovo-flex-5g.dts:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-primus.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-db845c.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts:&wifi {
arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb2210-rb1.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb4210-rb2.dts:&wifi {

The list doesn't include e.g. PIxel 2-3-4-5 or some other phones which
are still on their way for the proper mainline support.
--
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v5 2/2] iio: adc: adding support for PAC193x
From: Marius.Cristea @ 2024-04-05 12:53 UTC (permalink / raw)
  To: jic23, Conor.Dooley
  Cc: linux, jdelvare, linux-iio, devicetree, lars, linux-kernel,
	linux-hwmon, krzysztof.kozlowski+dt, robh+dt, conor+dt
In-Reply-To: <20240405-embellish-bonnet-ab5f10560d93@wendy>

Hi Conor,

    Thanks for reporting the bug. I have detect it and I'm already
working on a patch for it.

Thanks,
Marius

On Fri, 2024-04-05 at 13:41 +0100, Conor Dooley wrote:
> On Sat, Feb 24, 2024 at 07:15:59PM +0000, Jonathan Cameron wrote:
> > On Thu, 22 Feb 2024 18:42:06 +0200
> > <marius.cristea@microchip.com> wrote:
> > 
> > > From: Marius Cristea <marius.cristea@microchip.com>
> > > 
> > > This is the iio driver for Microchip
> > > PAC193X series of Power Monitor with Accumulator chip family.
> > > 
> > > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> > So I had a few comments on this, but nothing that can't be cleaned
> > up later.
> > + I'll fix the thing the bots didn't like on the bindings.
> > 
> > Series applied to the togreg branch of iio.git and pushed out
> > as testing for 0-day to take a look at it.
> 
> I tested this out on v6.9-rc2 and prompted a backtrace when collectd
> started running:
>         ------------[ cut here ]------------
>         UBSAN: array-index-out-of-bounds in
> /home/conor/stuff/linux/drivers/iio/adc/pac1934.c:857:25
>         index 7 is out of range for type 'u32 [4]'
>         CPU: 1 PID: 179 Comm: iiod Not tainted 6.9.0-rc2-dirty #1
>         Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
>         Call Trace:
>         [<ffffffff80006bba>] dump_backtrace+0x28/0x30
>         [<ffffffff80bd67d8>] show_stack+0x38/0x44
>         [<ffffffff80be7820>] dump_stack_lvl+0x6e/0x9a
>         [<ffffffff80be7864>] dump_stack+0x18/0x20
>         [<ffffffff80be1452>] ubsan_epilogue+0x10/0x46
>         [<ffffffff80615358>] __ubsan_handle_out_of_bounds+0x6a/0x78
>         [<ffffffff80981f3a>] pac1934_read_raw+0x20c/0x34c
>         [<ffffffff80977c4c>] iio_read_channel_info+0x5c/0xbe
>         [<ffffffff8073516e>] dev_attr_show+0x1c/0x4a
>         [<ffffffff80387292>] sysfs_kf_seq_show+0x80/0xcc
>         [<ffffffff80385b12>] kernfs_seq_show+0x3c/0x4a
>         [<ffffffff8031e3d8>] seq_read_iter+0x136/0x2e4
>         [<ffffffff80385cde>] kernfs_fop_read_iter+0x38/0x16a
>         [<ffffffff802e904a>] vfs_read+0x1be/0x2ba
>         [<ffffffff802e9c48>] ksys_read+0x64/0xd2
>         [<ffffffff802e9cd6>] __riscv_sys_read+0x20/0x28
>         [<ffffffff80be838a>] do_trap_ecall_u+0xee/0x204
>         [<ffffffff80bf57d0>] ret_from_exception+0x0/0x64
>         ---[ end trace ]---
> 
> The device itself only has 4 channels, but in sysfs there are "fake"
> channels for the average voltages and currents too. UBSAN points at:
>         case PAC1934_VSENSE_AVG_4_ADDR:
>                 *val = PAC1934_MAX_VSENSE_RSHIFTED_BY_16B;
>                 if (chan->scan_type.sign == 'u')
>                         *val2 = info->shunts[channel];
>                 else
>                         *val2 = info->shunts[channel] >> 1;
>                 return IIO_VAL_FRACTIONAL;
> 
> And info->shunts is only valid for the 4 real channels, so I guess
> the
> averaged channels probably need to do a [channel - 4] or similar. I
> dunno if that relation between averaged channels and their "real"
> counterparts is fixed or if different pac devices need different
> values,
> but on my system here that would work.
> 
> I do quite like that UBSAN points out the line in question :)
> 
> Cheers,
> Conor.


^ permalink raw reply

* [PATCH] arm64: dts: mediatek: mt7981: add I2C controller
From: Rafał Miłecki @ 2024-04-05 12:55 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

MT7981 has one on-SoC I2C controller that differs from recent Mediatek
blocks by having a different SLAVE_ADDR register offset (thus a custom
binding compatible string).

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
index 8a6263cc569c..2d7f91196e64 100644
--- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
@@ -94,6 +94,23 @@ pwm@10048000 {
 			#pwm-cells = <2>;
 		};
 
+		i2c@11007000 {
+			compatible = "mediatek,mt7981-i2c";
+			reg = <0 0x11007000 0 0x1000>,
+			      <0 0x10217080 0 0x80>;
+			interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&infracfg CLK_INFRA_I2C0_CK>,
+				 <&infracfg CLK_INFRA_AP_DMA_CK>,
+				 <&infracfg CLK_INFRA_I2C_MCK_CK>,
+				 <&infracfg CLK_INFRA_I2C_PCK_CK>;
+			clock-names = "main", "dma", "arb", "pmic";
+			clock-div = <1>;
+			clock-frequency = <100000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		pio: pinctrl@11d00000 {
 			compatible = "mediatek,mt7981-pinctrl";
 			reg = <0 0x11d00000 0 0x1000>,
-- 
2.35.3


^ permalink raw reply related

* Re: [RESEND PATCH v9 2/4] dt-bindings: stm32: update DT bingding for stm32mp25
From: Gabriel FERNANDEZ @ 2024-04-05 12:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Michael Turquette, Stephen Boyd, Krzysztof Kozlowski,
	Conor Dooley, Maxime Coquelin, Alexandre Torgue, Philipp Zabel,
	linux-clk, devicetree, linux-stm32, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20240404135201.GA2320777-robh@kernel.org>


On 4/4/24 15:52, Rob Herring wrote:
> On Tue, Apr 02, 2024 at 02:53:10PM +0200, gabriel.fernandez@foss.st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>>
>> Now RCC driver use '.index' of clk_parent_data struct to define a parent.
>> The majority of parents are SCMI clocks, then dt-bindings must be fixed.
> This is an ABI change. Please make that clear and justify why that is
> okay. Changing a driver is not a valid reason. What about other drivers
> besides Linux?

As the SoC STM32MP25X is not yet official and it is not available

outside STMicroelectronics, it is not a issue to have ABI change
and I will upstream the driver in other component (TF-A, U-Boot
and OP-TEE) when binding and driver will be accepted and merged
in Linux repository to avoid binding divergence.

Today no other STM32MP25 RCC drivers are yet upstreamed.

Best Regards,

Gabriel

>> Fixes: b5be49db3d47 ("dt-bindings: stm32: add clocks and reset binding for stm32mp25 platform")
>>
> Should not have a blank line here.
ok
>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>> ---
> Please put version history for a patch within the patch here.

ok


>
>>   .../bindings/clock/st,stm32mp25-rcc.yaml      | 171 ++++++++++++++++--
>>   1 file changed, 155 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml b/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
>> index 7732e79a42b9..57bd4e7157bd 100644
>> --- a/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
>> @@ -38,22 +38,87 @@ properties:
>>         - description: CK_SCMI_MSI Low Power Internal oscillator (~ 4 MHz or ~ 16 MHz)
>>         - description: CK_SCMI_LSE Low Speed External oscillator (32 KHz)
>>         - description: CK_SCMI_LSI Low Speed Internal oscillator (~ 32 KHz)
>> -
>> -  clock-names:
>> -    items:
>> -      - const: hse
>> -      - const: hsi
>> -      - const: msi
>> -      - const: lse
>> -      - const: lsi
>> -
>> +      - description: CK_SCMI_HSE_DIV2 CK_SCMI_HSE divided by 2 (coud be gated)
>> +      - description: CK_SCMI_ICN_HS_MCU High Speed interconnect bus clock
>> +      - description: CK_SCMI_ICN_LS_MCU Low Speed interconnect bus clock
>> +      - description: CK_SCMI_ICN_SDMMC SDMMC interconnect bus clock
>> +      - description: CK_SCMI_ICN_DDR DDR interconnect bus clock
>> +      - description: CK_SCMI_ICN_DISPLAY Display interconnect bus clock
>> +      - description: CK_SCMI_ICN_HSL HSL interconnect bus clock
>> +      - description: CK_SCMI_ICN_NIC NIC interconnect bus clock
>> +      - description: CK_SCMI_ICN_VID Video interconnect bus clock
>> +      - description: CK_SCMI_FLEXGEN_07 flexgen clock 7
>> +      - description: CK_SCMI_FLEXGEN_08 flexgen clock 8
>> +      - description: CK_SCMI_FLEXGEN_09 flexgen clock 9
>> +      - description: CK_SCMI_FLEXGEN_10 flexgen clock 10
>> +      - description: CK_SCMI_FLEXGEN_11 flexgen clock 11
>> +      - description: CK_SCMI_FLEXGEN_12 flexgen clock 12
>> +      - description: CK_SCMI_FLEXGEN_13 flexgen clock 13
>> +      - description: CK_SCMI_FLEXGEN_14 flexgen clock 14
>> +      - description: CK_SCMI_FLEXGEN_15 flexgen clock 15
>> +      - description: CK_SCMI_FLEXGEN_16 flexgen clock 16
>> +      - description: CK_SCMI_FLEXGEN_17 flexgen clock 17
>> +      - description: CK_SCMI_FLEXGEN_18 flexgen clock 18
>> +      - description: CK_SCMI_FLEXGEN_19 flexgen clock 19
>> +      - description: CK_SCMI_FLEXGEN_20 flexgen clock 20
>> +      - description: CK_SCMI_FLEXGEN_21 flexgen clock 21
>> +      - description: CK_SCMI_FLEXGEN_22 flexgen clock 22
>> +      - description: CK_SCMI_FLEXGEN_23 flexgen clock 23
>> +      - description: CK_SCMI_FLEXGEN_24 flexgen clock 24
>> +      - description: CK_SCMI_FLEXGEN_25 flexgen clock 25
>> +      - description: CK_SCMI_FLEXGEN_26 flexgen clock 26
>> +      - description: CK_SCMI_FLEXGEN_27 flexgen clock 27
>> +      - description: CK_SCMI_FLEXGEN_28 flexgen clock 28
>> +      - description: CK_SCMI_FLEXGEN_29 flexgen clock 29
>> +      - description: CK_SCMI_FLEXGEN_30 flexgen clock 30
>> +      - description: CK_SCMI_FLEXGEN_31 flexgen clock 31
>> +      - description: CK_SCMI_FLEXGEN_32 flexgen clock 32
>> +      - description: CK_SCMI_FLEXGEN_33 flexgen clock 33
>> +      - description: CK_SCMI_FLEXGEN_34 flexgen clock 34
>> +      - description: CK_SCMI_FLEXGEN_35 flexgen clock 35
>> +      - description: CK_SCMI_FLEXGEN_36 flexgen clock 36
>> +      - description: CK_SCMI_FLEXGEN_37 flexgen clock 37
>> +      - description: CK_SCMI_FLEXGEN_38 flexgen clock 38
>> +      - description: CK_SCMI_FLEXGEN_39 flexgen clock 39
>> +      - description: CK_SCMI_FLEXGEN_40 flexgen clock 40
>> +      - description: CK_SCMI_FLEXGEN_41 flexgen clock 41
>> +      - description: CK_SCMI_FLEXGEN_42 flexgen clock 42
>> +      - description: CK_SCMI_FLEXGEN_43 flexgen clock 43
>> +      - description: CK_SCMI_FLEXGEN_44 flexgen clock 44
>> +      - description: CK_SCMI_FLEXGEN_45 flexgen clock 45
>> +      - description: CK_SCMI_FLEXGEN_46 flexgen clock 46
>> +      - description: CK_SCMI_FLEXGEN_47 flexgen clock 47
>> +      - description: CK_SCMI_FLEXGEN_48 flexgen clock 48
>> +      - description: CK_SCMI_FLEXGEN_49 flexgen clock 49
>> +      - description: CK_SCMI_FLEXGEN_50 flexgen clock 50
>> +      - description: CK_SCMI_FLEXGEN_51 flexgen clock 51
>> +      - description: CK_SCMI_FLEXGEN_52 flexgen clock 52
>> +      - description: CK_SCMI_FLEXGEN_53 flexgen clock 53
>> +      - description: CK_SCMI_FLEXGEN_54 flexgen clock 54
>> +      - description: CK_SCMI_FLEXGEN_55 flexgen clock 55
>> +      - description: CK_SCMI_FLEXGEN_56 flexgen clock 56
>> +      - description: CK_SCMI_FLEXGEN_57 flexgen clock 57
>> +      - description: CK_SCMI_FLEXGEN_58 flexgen clock 58
>> +      - description: CK_SCMI_FLEXGEN_59 flexgen clock 59
>> +      - description: CK_SCMI_FLEXGEN_60 flexgen clock 60
>> +      - description: CK_SCMI_FLEXGEN_61 flexgen clock 61
>> +      - description: CK_SCMI_FLEXGEN_62 flexgen clock 62
>> +      - description: CK_SCMI_FLEXGEN_63 flexgen clock 63
>> +      - description: CK_SCMI_ICN_APB1 Peripheral bridge 1
>> +      - description: CK_SCMI_ICN_APB2 Peripheral bridge 2
>> +      - description: CK_SCMI_ICN_APB3 Peripheral bridge 3
>> +      - description: CK_SCMI_ICN_APB4 Peripheral bridge 4
>> +      - description: CK_SCMI_ICN_APBDBG Peripheral bridge for degub
>> +      - description: CK_SCMI_TIMG1 Peripheral bridge for timer1
>> +      - description: CK_SCMI_TIMG2 Peripheral bridge for timer2
>> +      - description: CK_SCMI_PLL3 PLL3 clock
>> +      - description: clk_dsi_txbyte DSI byte clock
> Need a blank line here.
>
>>   required:
>>     - compatible
>>     - reg
>>     - '#clock-cells'
>>     - '#reset-cells'
>>     - clocks
>> -  - clock-names
>>   
>>   additionalProperties: false
>>   
>> @@ -66,11 +131,85 @@ examples:
>>           reg = <0x44200000 0x10000>;
>>           #clock-cells = <1>;
>>           #reset-cells = <1>;
>> -        clock-names = "hse", "hsi", "msi", "lse", "lsi";
>> -        clocks = <&scmi_clk CK_SCMI_HSE>,
>> -                 <&scmi_clk CK_SCMI_HSI>,
>> -                 <&scmi_clk CK_SCMI_MSI>,
>> -                 <&scmi_clk CK_SCMI_LSE>,
>> -                 <&scmi_clk CK_SCMI_LSI>;
>> +        clocks =  <&scmi_clk CK_SCMI_HSE>,
>> +                  <&scmi_clk CK_SCMI_HSI>,
>> +                  <&scmi_clk CK_SCMI_MSI>,
>> +                  <&scmi_clk CK_SCMI_LSE>,
>> +                  <&scmi_clk CK_SCMI_LSI>,
>> +                  <&scmi_clk CK_SCMI_HSE_DIV2>,
>> +                  <&scmi_clk CK_SCMI_ICN_HS_MCU>,
>> +                  <&scmi_clk CK_SCMI_ICN_LS_MCU>,
>> +                  <&scmi_clk CK_SCMI_ICN_SDMMC>,
>> +                  <&scmi_clk CK_SCMI_ICN_DDR>,
>> +                  <&scmi_clk CK_SCMI_ICN_DISPLAY>,
>> +                  <&scmi_clk CK_SCMI_ICN_HSL>,
>> +                  <&scmi_clk CK_SCMI_ICN_NIC>,
>> +                  <&scmi_clk CK_SCMI_ICN_VID>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_07>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_08>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_09>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_10>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_11>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_12>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_13>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_14>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_15>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_16>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_17>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_18>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_19>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_20>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_21>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_22>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_23>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_24>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_25>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_26>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_27>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_28>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_29>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_30>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_31>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_32>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_33>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_34>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_35>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_36>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_37>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_38>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_39>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_40>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_41>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_42>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_43>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_44>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_45>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_46>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_47>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_48>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_49>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_50>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_51>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_52>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_53>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_54>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_55>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_56>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_57>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_58>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_59>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_60>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_61>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_62>,
>> +                  <&scmi_clk CK_SCMI_FLEXGEN_63>,
>> +                  <&scmi_clk CK_SCMI_ICN_APB1>,
>> +                  <&scmi_clk CK_SCMI_ICN_APB2>,
>> +                  <&scmi_clk CK_SCMI_ICN_APB3>,
>> +                  <&scmi_clk CK_SCMI_ICN_APB4>,
>> +                  <&scmi_clk CK_SCMI_ICN_APBDBG>,
>> +                  <&scmi_clk CK_SCMI_TIMG1>,
>> +                  <&scmi_clk CK_SCMI_TIMG2>,
>> +                  <&scmi_clk CK_SCMI_PLL3>,
>> +                  <&clk_dsi_txbyte>;
>>       };
>>   ...
>> -- 
>> 2.25.1
>>

^ permalink raw reply

* Re: [RESEND PATCH v9 2/4] dt-bindings: stm32: update DT bingding for stm32mp25
From: Gabriel FERNANDEZ @ 2024-04-05 12:54 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
	Alexandre Torgue, Philipp Zabel
  Cc: linux-clk, devicetree, linux-stm32, linux-arm-kernel,
	linux-kernel
In-Reply-To: <e70dc513-df9f-4b99-b9d9-7ebaf83e8f3e@linaro.org>


On 4/5/24 09:12, Krzysztof Kozlowski wrote:
> On 02/04/2024 14:53, gabriel.fernandez@foss.st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>>
>> Now RCC driver use '.index' of clk_parent_data struct to define a parent.
>> The majority of parents are SCMI clocks, then dt-bindings must be fixed.
>>
>> Fixes: b5be49db3d47 ("dt-bindings: stm32: add clocks and reset binding for stm32mp25 platform")
> And except what Rob said, this does not look as a fix. How ABI break
> could be a fix and what is even to fix here? Please describe the
> observable bug, how it manifests itself and what is exactly the fix for
> that bug.
As I replied to Rob, there are no RCC STM32MP25 drivers already upstreamed.

However, in my series, the DT binding was merged even though Stephen 
made some

important remarks that needed to be taken into account.

That's why I proposed a fix to update the documentation.

To be sure, how would you like me to proceed?

Best Regards,

Gabriel

>
> Best regards,
> Krzysztof
>

^ permalink raw reply

* Re: [PATCH v3 6/9] drm: xlnx: zynqmp_dpsub: Set input live format
From: Tomi Valkeinen @ 2024-04-05 12:56 UTC (permalink / raw)
  To: Anatoliy Klymenko
  Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
	linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
	Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-6-d5090d796b7e@amd.com>

On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Program live video input format according to selected media bus format.
> 
> In the bridge mode of operation, DPSUB is connected to FPGA CRTC which
> almost certainly supports a single media bus format as its output. Expect
> this to be delivered via the new bridge atomic state. Program DPSUB
> registers accordingly. Update zynqmp_disp_layer_set_format() API to fit
> both live and non-live layer types.
> 
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
>   drivers/gpu/drm/xlnx/zynqmp_disp.c | 66 +++++++++++++++++++++++++-------------
>   drivers/gpu/drm/xlnx/zynqmp_disp.h |  2 +-
>   drivers/gpu/drm/xlnx/zynqmp_dp.c   | 13 +++++---
>   drivers/gpu/drm/xlnx/zynqmp_kms.c  |  2 +-
>   4 files changed, 55 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 0c2b3f4bffa6..a385d22d428e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -436,19 +436,28 @@ static void zynqmp_disp_avbuf_set_format(struct zynqmp_disp *disp,
>   					 const struct zynqmp_disp_format *fmt)
>   {
>   	unsigned int i;
> -	u32 val;
> +	u32 val, reg;
>   
> -	val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> -	val &= zynqmp_disp_layer_is_video(layer)
> -	    ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> -	    : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> -	val |= fmt->buf_fmt;
> -	zynqmp_disp_avbuf_write(disp, ZYNQMP_DISP_AV_BUF_FMT, val);
> +	layer->disp_fmt = fmt;
> +	if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
> +		reg = ZYNQMP_DISP_AV_BUF_FMT;
> +		val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> +		val &= zynqmp_disp_layer_is_video(layer)
> +		    ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> +		    : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> +		val |= fmt->buf_fmt;
> +	} else {
> +		reg = zynqmp_disp_layer_is_video(layer)
> +		    ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG
> +		    : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG;
> +		val = fmt->buf_fmt;
> +	}
> +	zynqmp_disp_avbuf_write(disp, reg, val);

Just write the registers inside the above if-else blocks.

>   
>   	for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; i++) {
> -		unsigned int reg = zynqmp_disp_layer_is_video(layer)
> -				 ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> -				 : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
> +		reg = zynqmp_disp_layer_is_video(layer)
> +		    ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> +		    : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
>   
>   		zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]);
>   	}
> @@ -902,25 +911,33 @@ static void zynqmp_disp_audio_disable(struct zynqmp_disp *disp)
>    */
>   
>   /**
> - * zynqmp_disp_layer_find_format - Find format information for a DRM format
> + * zynqmp_disp_layer_find_format - Find format information for a DRM or media
> + * bus format
>    * @layer: The layer
> - * @drm_fmt: DRM format to search
> + * @drm_or_bus_format: DRM or media bus format
>    *
>    * Search display subsystem format information corresponding to the given DRM
> - * format @drm_fmt for the @layer, and return a pointer to the format
> - * descriptor.
> + * or media bus format @drm_or_bus_format for the @layer, and return a pointer
> + * to the format descriptor. Search key choice depends on @layer mode, for live
> + * layers search is done by zynqmp_disp_format.bus_fmt, and for non-live layers
> + * zynqmp_disp_format.drm_fmt is used.

Here also I recommend creating separate funcs for the fourcc and mbus 
versions. They are different types, even if they happen to fit into u32.

>    *
>    * Return: A pointer to the format descriptor if found, NULL otherwise
>    */
>   static const struct zynqmp_disp_format *
>   zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
> -			      u32 drm_fmt)
> +			      u32 drm_or_bus_format)
>   {
>   	unsigned int i;
> +	const struct zynqmp_disp_format *disp_format;
>   
>   	for (i = 0; i < layer->info->num_formats; i++) {
> -		if (layer->info->formats[i].drm_fmt == drm_fmt)
> -			return &layer->info->formats[i];
> +		disp_format = &layer->info->formats[i];
> +		if ((layer->mode == ZYNQMP_DPSUB_LAYER_LIVE &&
> +		     disp_format->bus_fmt == drm_or_bus_format) ||
> +		    (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE &&
> +		     disp_format->drm_fmt == drm_or_bus_format))
> +			return disp_format;
>   	}
>   
>   	return NULL;
> @@ -992,20 +1009,25 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
>   /**
>    * zynqmp_disp_layer_set_format - Set the layer format
>    * @layer: The layer
> - * @info: The format info
> + * @drm_or_bus_format: DRM or media bus format
>    *
>    * Set the format for @layer to @info. The layer must be disabled.
>    */
>   void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> -				  const struct drm_format_info *info)
> +				  u32 drm_or_bus_format)

And here, with a quick look, a separate function would be fine.

  Tomi

>   {
>   	unsigned int i;
>   
> -	layer->disp_fmt = zynqmp_disp_layer_find_format(layer, info->format);
> -	layer->drm_fmt = info;
> +	layer->disp_fmt = zynqmp_disp_layer_find_format(layer, drm_or_bus_format);
> +	if (WARN_ON(!layer->disp_fmt))
> +		return;
>   
>   	zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>   
> +	layer->drm_fmt = drm_format_info(layer->disp_fmt->drm_fmt);
> +	if (!layer->drm_fmt)
> +		return;
> +
>   	if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
>   		return;
>   
> @@ -1013,7 +1035,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
>   	 * Set pconfig for each DMA channel to indicate they're part of a
>   	 * video group.
>   	 */
> -	for (i = 0; i < info->num_planes; i++) {
> +	for (i = 0; i < layer->drm_fmt->num_planes; i++) {
>   		struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
>   		struct xilinx_dpdma_peripheral_config pconfig = {
>   			.video_group = true,
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 88c285a12e23..9f9a5f50ffbc 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -55,7 +55,7 @@ u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
>   void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
>   void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
>   void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> -				  const struct drm_format_info *info);
> +				  u32 drm_or_bus_format);
>   int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
>   			     struct drm_plane_state *state);
>   
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index e3b9eb3d9273..200e63636006 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1299,15 +1299,20 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
>   				  struct drm_bridge_state *old_bridge_state)
>   {
>   	struct zynqmp_disp_layer *layer;
> -	const struct drm_format_info *info;
> +	struct drm_bridge_state *bridge_state;
> +	u32 bus_fmt;
>   
>   	layer = zynqmp_dp_disp_connected_live_layer(dp);
>   	if (!layer)
>   		return;
>   
> -	/* TODO: Make the format configurable. */
> -	info = drm_format_info(DRM_FORMAT_YUV422);
> -	zynqmp_disp_layer_set_format(layer, info);
> +	bridge_state = drm_atomic_get_new_bridge_state(old_bridge_state->base.state,
> +						       old_bridge_state->bridge);
> +	if (WARN_ON(!bridge_state))
> +		return;
> +
> +	bus_fmt = bridge_state->input_bus_cfg.format;
> +	zynqmp_disp_layer_set_format(layer, bus_fmt);
>   	zynqmp_disp_layer_enable(layer);
>   
>   	if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index bf9fba01df0e..d96b3f3f2e3a 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -111,7 +111,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
>   		if (old_state->fb)
>   			zynqmp_disp_layer_disable(layer);
>   
> -		zynqmp_disp_layer_set_format(layer, new_state->fb->format);
> +		zynqmp_disp_layer_set_format(layer, new_state->fb->format->format);
>   	}
>   
>   	zynqmp_disp_layer_update(layer, new_state);
> 


^ permalink raw reply

* [PATCH 1/2] arm64: dts: mediatek: mt7988: add PWM controller
From: Rafał Miłecki @ 2024-04-05 12:55 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

MT7988 has on-SoC controller that can control up to 8 PWM interfaces.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
index 3eb5396dea22..27098f724b7a 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
@@ -105,6 +105,25 @@ clock-controller@1001e000 {
 			#clock-cells = <1>;
 		};
 
+		pwm@10048000 {
+			compatible = "mediatek,mt7988-pwm";
+			reg = <0 0x10048000 0 0x1000>;
+			clocks = <&infracfg CLK_INFRA_66M_PWM_BCK>,
+				 <&infracfg CLK_INFRA_66M_PWM_HCK>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK1>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK2>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK3>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK4>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK5>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK6>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK7>,
+				 <&infracfg CLK_INFRA_66M_PWM_CK8>;
+			clock-names = "top", "main", "pwm1", "pwm2", "pwm3",
+				      "pwm4", "pwm5", "pwm6", "pwm7", "pwm8";
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
 		usb@11190000 {
 			compatible = "mediatek,mt7988-xhci", "mediatek,mtk-xhci";
 			reg = <0 0x11190000 0 0x2e00>,
-- 
2.35.3


^ permalink raw reply related

* [PATCH 2/2] arm64: dts: mediatek: mt7988: add I2C controllers
From: Rafał Miłecki @ 2024-04-05 12:55 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	Rafał Miłecki
In-Reply-To: <20240405125549.11972-1-zajec5@gmail.com>

From: Rafał Miłecki <rafal@milecki.pl>

MT7988 has three on-SoC I2C controllers that are the same hardware
blocks as already noticed on MT7981 chipsets.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 45 +++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
index 27098f724b7a..b4dc81881cc7 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
@@ -124,6 +124,51 @@ pwm@10048000 {
 			status = "disabled";
 		};
 
+		i2c@11003000 {
+			compatible = "mediatek,mt7981-i2c";
+			reg = <0 0x11003000 0 0x1000>,
+			      <0 0x10217080 0 0x80>;
+			interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&infracfg CLK_INFRA_I2C_BCK>,
+				 <&infracfg CLK_INFRA_66M_AP_DMA_BCK>;
+			clock-names = "main", "dma";
+			clock-div = <1>;
+			clock-frequency = <100000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c@11004000 {
+			compatible = "mediatek,mt7981-i2c";
+			reg = <0 0x11004000 0 0x1000>,
+			      <0 0x10217100 0 0x80>;
+			interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&infracfg CLK_INFRA_I2C_BCK>,
+				 <&infracfg CLK_INFRA_66M_AP_DMA_BCK>;
+			clock-names = "main", "dma";
+			clock-div = <1>;
+			clock-frequency = <100000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c@11005000 {
+			compatible = "mediatek,mt7981-i2c";
+			reg = <0 0x11005000 0 0x1000>,
+			      <0 0x10217180 0 0x80>;
+			interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&infracfg CLK_INFRA_I2C_BCK>,
+				 <&infracfg CLK_INFRA_66M_AP_DMA_BCK>;
+			clock-names = "main", "dma";
+			clock-div = <1>;
+			clock-frequency = <100000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		usb@11190000 {
 			compatible = "mediatek,mt7988-xhci", "mediatek,mtk-xhci";
 			reg = <0 0x11190000 0 0x2e00>,
-- 
2.35.3


^ permalink raw reply related

* Re: [RESEND v7 14/37] clk: Compatible with narrow registers
From: Geert Uytterhoeven @ 2024-04-05 12:56 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
	Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
	John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
	Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
	Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
	Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
	Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
	Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
	Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
	Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
	Laurent Pinchart, linux-ide, devicetree, linux-kernel,
	linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
	linux-fbdev
In-Reply-To: <9c1d56d37f5d3780d3c506ae680133b6bdaa5fdc.1712207606.git.ysato@users.sourceforge.jp>

Hi Sato-san,

On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> divider and gate only support 32-bit registers.
> Older hardware uses narrower registers, so I want to be able to handle
> 8-bit and 16-bit wide registers.
>
> Seven clk_divider flags are used, and if I add flags for 8bit access and
> 16bit access, 8bit will not be enough, so I expanded it to u16.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Thanks for the update!

> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -26,20 +26,38 @@
>   * parent - fixed parent.  No clk_set_parent support
>   */
>
> -static inline u32 clk_div_readl(struct clk_divider *divider)
> -{
> -       if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> -               return ioread32be(divider->reg);
> -
> -       return readl(divider->reg);
> +static inline u32 clk_div_read(struct clk_divider *divider)
> +{
> +       if (divider->flags & CLK_DIVIDER_REG_8BIT)

When you need curly braces in one branch of an if/else statement,
please use curly braces in all branches (everywhere).

> +               return readb(divider->reg);
> +       else if (divider->flags & CLK_DIVIDER_REG_16BIT) {
> +               if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> +                       return ioread16be(divider->reg);
> +               else
> +                       return readw(divider->reg);
> +       } else {
> +               if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> +                       return ioread32be(divider->reg);
> +               else
> +                       return readl(divider->reg);
> +       }
>  }

> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c

> @@ -137,12 +155,30 @@ struct clk_hw *__clk_hw_register_gate(struct device *dev,
>         struct clk_init_data init = {};
>         int ret = -EINVAL;
>
> +       /* validate register size option and bit_idx */
>         if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
>                 if (bit_idx > 15) {
>                         pr_err("gate bit exceeds LOWORD field\n");
>                         return ERR_PTR(-EINVAL);
>                 }
>         }
> +       if (clk_gate_flags & CLK_GATE_REG_16BIT) {
> +               if (bit_idx > 15) {
> +                       pr_err("gate bit exceeds 16 bits\n");
> +                       return ERR_PTR(-EINVAL);
> +               }
> +       }
> +       if (clk_gate_flags & CLK_GATE_REG_8BIT) {
> +               if (bit_idx > 7) {
> +                       pr_err("gate bit exceeds 8 bits\n");
> +                       return ERR_PTR(-EINVAL);
> +               }
> +       }
> +       if ((clk_gate_flags & CLK_GATE_HIWORD_MASK) &&

If you use parentheses around "a & b" here...

> +           clk_gate_flags & (CLK_GATE_REG_8BIT | CLK_GATE_REG_16BIT)) {

please add parentheses here, too.

> +               pr_err("HIWORD_MASK required 32-bit register\n");
> +               return ERR_PTR(-EINVAL);
> +       }
>
>         /* allocate the gate */
>         gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 4a537260f655..eaa6ff1d0b2e 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np);
>   * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used for
>   *     the gate register.  Setting this flag makes the register accesses big
>   *     endian.
> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for
> + *     the gate register.  Setting this flag makes the register accesses 8bit.
> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for
> + *     the gate register.  Setting this flag makes the register accesses 16bit.
>   */
>  struct clk_gate {
>         struct clk_hw hw;
>         void __iomem    *reg;
>         u8              bit_idx;
> -       u8              flags;
> +       u32             flags;

(from my comments on v6)
There is no need to increase the size of the flags field for the gate clock.


>         spinlock_t      *lock;
>  };
>

> @@ -675,13 +681,17 @@ struct clk_div_table {
>   * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
>   *     for the divider register.  Setting this flag makes the register accesses
>   *     big endian.
> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
> + *     the gate register.  Setting this flag makes the register accesses 8bit.
> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
> + *     the gate register.  Setting this flag makes the register accesses 16bit.
>   */
>  struct clk_divider {
>         struct clk_hw   hw;
>         void __iomem    *reg;
>         u8              shift;
>         u8              width;
> -       u8              flags;
> +       u16             flags;
>         const struct clk_div_table      *table;
>         spinlock_t      *lock;
>  };

> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
>                 struct device_node *np, const char *name,
>                 const char *parent_name, const struct clk_hw *parent_hw,
>                 const struct clk_parent_data *parent_data, unsigned long flags,
> -               void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
> +               void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,

"u16 clk_divider_flags", to match clk_divider.flags.

>                 const struct clk_div_table *table, spinlock_t *lock);
>  struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
>                 struct device_node *np, const char *name,
>                 const char *parent_name, const struct clk_hw *parent_hw,
>                 const struct clk_parent_data *parent_data, unsigned long flags,
> -               void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
> +               void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,

Likewise.

>                 const struct clk_div_table *table, spinlock_t *lock);
>  struct clk *clk_register_divider_table(struct device *dev, const char *name,
>                 const char *parent_name, unsigned long flags,
>                 void __iomem *reg, u8 shift, u8 width,
> -               u8 clk_divider_flags, const struct clk_div_table *table,
> +               u32 clk_divider_flags, const struct clk_div_table *table,

Likewise.

>                 spinlock_t *lock);
>  /**
>   * clk_register_divider - register a divider clock with the clock framework

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 3/3] of: Use scope based of_node_put() cleanups
From: Rob Herring @ 2024-04-05 13:00 UTC (permalink / raw)
  To: Saravana Kannan; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <CAGETcx8Wd5OsHWiGSASWkQQtof0D-ScwYsvq9hWizV3DFC27gA@mail.gmail.com>

On Thu, Apr 4, 2024 at 6:22 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
> >
> > Use the relatively new scope based of_node_put() cleanup to simplify
> > function exit handling. Doing so reduces the chances of forgetting an
> > of_node_put() and simplifies error paths by avoiding the need for goto
> > statements.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/of/address.c  | 60 ++++++++++++++++-----------------------------------
> >  drivers/of/property.c | 22 ++++++-------------
> >  2 files changed, 26 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index ae46a3605904..f7b2d535a6d1 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -491,7 +491,6 @@ static u64 __of_translate_address(struct device_node *dev,
> >                                   const __be32 *in_addr, const char *rprop,
> >                                   struct device_node **host)
> >  {
> > -       struct device_node *parent = NULL;
> >         struct of_bus *bus, *pbus;
> >         __be32 addr[OF_MAX_ADDR_CELLS];
> >         int na, ns, pna, pns;
> > @@ -504,7 +503,7 @@ static u64 __of_translate_address(struct device_node *dev,
> >
> >         *host = NULL;
> >         /* Get parent & match bus type */
> > -       parent = get_parent(dev);
> > +       struct device_node *parent __free(device_node) = get_parent(dev);
>
> Can we leave the variable definition where it was? We generally define
> all the variables up top. So, defining the one variable in the middle
> feels weird. I at least get when we do this inside for/if blocks. But
> randomly in the middle feels weird.

There's an 'of_node_get(dev);' before this. Ordering wise, we need to
hold the ref on the child before we get its parent. I suppose I can
also convert that to use the cleanups. I'll have to add another local
ptr to do that though.

>
> Similar comments in other places. Since both kfree() and of_put() can
> both handle NULL pointers, I'd be surprised if we HAVE to combine
> these lines.

https://lore.kernel.org/all/CAHk-=wgRHiV5VSxtfXA4S6aLUmcQYEuB67u3BJPJPtuESs1JyA@mail.gmail.com/

^ permalink raw reply

* Re: [RESEND v7 14/37] clk: Compatible with narrow registers
From: Damien Le Moal @ 2024-04-05 13:07 UTC (permalink / raw)
  To: Geert Uytterhoeven, Yoshinori Sato
  Cc: linux-sh, Niklas Cassel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Michael Turquette, Stephen Boyd, David Airlie,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
	Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
	John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
	Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
	Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
	Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
	Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
	Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
	Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
	Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
	Laurent Pinchart, linux-ide, devicetree, linux-kernel,
	linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
	linux-fbdev
In-Reply-To: <CAMuHMdVXvPW+3-sY2XPQ2aMcTZkK9zoMnxWeZ+PRB+VRgGszdQ@mail.gmail.com>

On 4/5/24 21:56, Geert Uytterhoeven wrote:
> Hi Sato-san,
> 
> On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
> <ysato@users.sourceforge.jp> wrote:
>> divider and gate only support 32-bit registers.
>> Older hardware uses narrower registers, so I want to be able to handle
>> 8-bit and 16-bit wide registers.
>>
>> Seven clk_divider flags are used, and if I add flags for 8bit access and
>> 16bit access, 8bit will not be enough, so I expanded it to u16.
>>
>> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> 
> Thanks for the update!
> 
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -26,20 +26,38 @@
>>   * parent - fixed parent.  No clk_set_parent support
>>   */
>>
>> -static inline u32 clk_div_readl(struct clk_divider *divider)
>> -{
>> -       if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> -               return ioread32be(divider->reg);
>> -
>> -       return readl(divider->reg);
>> +static inline u32 clk_div_read(struct clk_divider *divider)
>> +{
>> +       if (divider->flags & CLK_DIVIDER_REG_8BIT)
> 
> When you need curly braces in one branch of an if/else statement,
> please use curly braces in all branches (everywhere).
> 
>> +               return readb(divider->reg);
>> +       else if (divider->flags & CLK_DIVIDER_REG_16BIT) {

And no need for an else after a return...


>> +               if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> +                       return ioread16be(divider->reg);
>> +               else

and here.

>> +                       return readw(divider->reg);
>> +       } else {
>> +               if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> +                       return ioread32be(divider->reg);
>> +               else

here too.

>> +                       return readl(divider->reg);
>> +       }
>>  }
> 
>> --- a/drivers/clk/clk-gate.c
>> +++ b/drivers/clk/clk-gate.c
> 
>> @@ -137,12 +155,30 @@ struct clk_hw *__clk_hw_register_gate(struct device *dev,
>>         struct clk_init_data init = {};
>>         int ret = -EINVAL;
>>
>> +       /* validate register size option and bit_idx */
>>         if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
>>                 if (bit_idx > 15) {
>>                         pr_err("gate bit exceeds LOWORD field\n");
>>                         return ERR_PTR(-EINVAL);
>>                 }
>>         }
>> +       if (clk_gate_flags & CLK_GATE_REG_16BIT) {
>> +               if (bit_idx > 15) {
>> +                       pr_err("gate bit exceeds 16 bits\n");
>> +                       return ERR_PTR(-EINVAL);
>> +               }
>> +       }
>> +       if (clk_gate_flags & CLK_GATE_REG_8BIT) {
>> +               if (bit_idx > 7) {
>> +                       pr_err("gate bit exceeds 8 bits\n");
>> +                       return ERR_PTR(-EINVAL);
>> +               }
>> +       }
>> +       if ((clk_gate_flags & CLK_GATE_HIWORD_MASK) &&
> 
> If you use parentheses around "a & b" here...
> 
>> +           clk_gate_flags & (CLK_GATE_REG_8BIT | CLK_GATE_REG_16BIT)) {
> 
> please add parentheses here, too.
> 
>> +               pr_err("HIWORD_MASK required 32-bit register\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>>
>>         /* allocate the gate */
>>         gate = kzalloc(sizeof(*gate), GFP_KERNEL);
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 4a537260f655..eaa6ff1d0b2e 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np);
>>   * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used for
>>   *     the gate register.  Setting this flag makes the register accesses big
>>   *     endian.
>> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for
>> + *     the gate register.  Setting this flag makes the register accesses 8bit.
>> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for
>> + *     the gate register.  Setting this flag makes the register accesses 16bit.
>>   */
>>  struct clk_gate {
>>         struct clk_hw hw;
>>         void __iomem    *reg;
>>         u8              bit_idx;
>> -       u8              flags;
>> +       u32             flags;
> 
> (from my comments on v6)
> There is no need to increase the size of the flags field for the gate clock.
> 
> 
>>         spinlock_t      *lock;
>>  };
>>
> 
>> @@ -675,13 +681,17 @@ struct clk_div_table {
>>   * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
>>   *     for the divider register.  Setting this flag makes the register accesses
>>   *     big endian.
>> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
>> + *     the gate register.  Setting this flag makes the register accesses 8bit.
>> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
>> + *     the gate register.  Setting this flag makes the register accesses 16bit.
>>   */
>>  struct clk_divider {
>>         struct clk_hw   hw;
>>         void __iomem    *reg;
>>         u8              shift;
>>         u8              width;
>> -       u8              flags;
>> +       u16             flags;
>>         const struct clk_div_table      *table;
>>         spinlock_t      *lock;
>>  };
> 
>> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
>>                 struct device_node *np, const char *name,
>>                 const char *parent_name, const struct clk_hw *parent_hw,
>>                 const struct clk_parent_data *parent_data, unsigned long flags,
>> -               void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> +               void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
> 
> "u16 clk_divider_flags", to match clk_divider.flags.
> 
>>                 const struct clk_div_table *table, spinlock_t *lock);
>>  struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
>>                 struct device_node *np, const char *name,
>>                 const char *parent_name, const struct clk_hw *parent_hw,
>>                 const struct clk_parent_data *parent_data, unsigned long flags,
>> -               void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> +               void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
> 
> Likewise.
> 
>>                 const struct clk_div_table *table, spinlock_t *lock);
>>  struct clk *clk_register_divider_table(struct device *dev, const char *name,
>>                 const char *parent_name, unsigned long flags,
>>                 void __iomem *reg, u8 shift, u8 width,
>> -               u8 clk_divider_flags, const struct clk_div_table *table,
>> +               u32 clk_divider_flags, const struct clk_div_table *table,
> 
> Likewise.
> 
>>                 spinlock_t *lock);
>>  /**
>>   * clk_register_divider - register a divider clock with the clock framework
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply

* Re: [PATCH v3 19/25] media: i2c: imx258: Change register settings for variants of the sensor
From: Dave Stevenson @ 2024-04-05 13:16 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Luigi311, 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: <Zg_Zl0Q2kEDJoQoe@kekkonen.localdomain>

Hi Sakari

On Fri, 5 Apr 2024 at 11:59, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> Hi Luis, Dave,
>
> On Thu, Apr 04, 2024 at 04:44:05PM -0600, Luigi311 wrote:
> > 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?
>
> Speaking of which---are you absolutely certain there are two variants of
> this sensor? Many sensors that have a different pixel pattern (PDAF pixels
> or a non-Bayer pattern) can produce Bayer data when condigured so. The fact
> that you have differing register configuration for the PDAF and non-PDAF
> cases suggests this may well be the case.

I had a discussion with our contact at Sony over the configuration,
and Soho Enterprises who made the module I have also consulted with
Sony (their main person is ex Sony himself).

There is a spec version field in the OTP which reflects the pixel
pattern. It has defined options of:
- HDR pattern
- Binning pattern
- mono
- non-PDAF
- HDR HDD

Sony can't release information on how to read that information from
the sensor OTP as it is contractually locked by contracts with Intel.
Whilst information obtained via other routes means I have checked it
on my module as HDR pattern whilst the Nautilus platform has the
non-PDAF variant, I'm not going to spoil our relationship with Sony by
releasing that.

It's possible that the Nautilus sensor will work happily with the
settings required for the PDAF variant, but I have no way of testing
that, and the registers in question are undocumented. Changing them
blindly isn't going to make any friends, and I doubt existing platform
users wish to rerun all their image quality tests on the sensor to
validate the change.

Unless Intel wish to release the information on reading the OTP, we
have no way of telling the variants apart but need different register
configurations. If there is a better way of handling that situation
than compatible strings, then I'm open to suggestions.

There's a short thread on libcamera-devel from back in 2022 where I
was looking into this [1]

  Dave

[1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-June/031449.html

> >
> > >> +  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);
> > >
> >
>
> --
> Regards,
>
> Sakari Ailus

^ permalink raw reply

* Re: [RESEND v7 23/37] dt-bindings: display: sm501 register definition helper
From: Geert Uytterhoeven @ 2024-04-05 13:25 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
	Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
	John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
	Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
	Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
	Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
	Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
	Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
	Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
	Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
	Laurent Pinchart, linux-ide, devicetree, linux-kernel,
	linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
	linux-fbdev
In-Reply-To: <dac98a697c8e850054f984964af62a209f241c83.1712207606.git.ysato@users.sourceforge.jp>

Hi Sato-san,

Thanks for your patch!

On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Miscellaneous Timing and Miscellaneous Control registers definition.

Please do not put raw register value definitions into DT bindings.

> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> --- /dev/null
> +++ b/include/dt-bindings/display/sm501.h

> +/* Miscellaneous timing */
> +#define SM501_MISC_TIMING_EX_HOLD_0    0
> +#define SM501_MISC_TIMING_EX_HOLD_16   1
> +#define SM501_MISC_TIMING_EX_HOLD_32   2
> +#define SM501_MISC_TIMING_EX_HOLD_48   3
> +#define SM501_MISC_TIMING_EX_HOLD_64   4
> +#define SM501_MISC_TIMING_EX_HOLD_80   5
> +#define SM501_MISC_TIMING_EX_HOLD_96   6
> +#define SM501_MISC_TIMING_EX_HOLD_112  7
> +#define SM501_MISC_TIMING_EX_HOLD_128  8
> +#define SM501_MISC_TIMING_EX_HOLD_144  9
> +#define SM501_MISC_TIMING_EX_HOLD_160  10
> +#define SM501_MISC_TIMING_EX_HOLD_176  11
> +#define SM501_MISC_TIMING_EX_HOLD_192  12
> +#define SM501_MISC_TIMING_EX_HOLD_208  13
> +#define SM501_MISC_TIMING_EX_HOLD_224  14
> +#define SM501_MISC_TIMING_EX_HOLD_240  15

E.g. these are used by the (not very descriptive) "ex" property:

     ex:
       $ref: /schemas/types.yaml#/definitions/uint32
       description: Extend bus holding time.

Please instead use an enum for the actual holding time ([ 0, 16, 32,
...]) in the DT bindings, and convert from actual holding time to
register value in the driver.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 01/18] PCI: endpoint: Introduce pci_epc_function_is_valid()
From: Niklas Cassel @ 2024-04-05 13:33 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
	Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
	Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
	Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240330041928.1555578-2-dlemoal@kernel.org>

On Sat, Mar 30, 2024 at 01:19:11PM +0900, Damien Le Moal wrote:
> Introduce the epc core helper function pci_epc_function_is_valid() to
> verify that an epc pointer, a physical function number and a virtual
> function number are all valid. This avoids repeating the code pattern:
> 
> if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> 	return err;
> 
> if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
> 	return err;
> 
> in many functions of the endpoint controller core code.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---

Reviewed-by: Niklas Cassel <cassel@kernel.org>

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree
From: Andrew Lunn @ 2024-04-05 13:35 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	David Jander
In-Reply-To: <Zg_NwfxLhzdCjN87@pengutronix.de>

> Do you have a pointer why setting the delays in the phy is preferred
> over setting them in the network driver? In the end this requires us
> to have the correct phy driver whereas setting them in the network
> driver would just work for any phy driver?

One reason is that nearly every other board does it in the PHY. This
is something i've been trying to standardize on for years.

Another point is that when doing it in the MAC, most MAC drivers get
it wrong. RGMII needs 2ns delays on the clock lines. That delay can be
provided by the board, making the clock lines longer. Or the MAC or
the PHY can add the delays. phy-mode in DT tells you about what the
board requires. Your board does not have extra long clock lines, so
you need rgmii-id. If the MAC decides to implement the delay, it
should modify the value passed to the PHY to be rgmii, to indicate it
has added the delays, and the PHY should not. This is what many MAC
drivers get wrong, they don't do the masking. By standardizing on the
PHY doing the delay, we avoid this, keeping the MAC driver simple, and
probably bug free in this respect.

There is admittedly some historical confusion here. The design is not
the best. If would of been much better if the design would have both
phy-mode and mac-mode.

As for using genphy, yes it might work, but there is no real
guarantee. It is always best you drive the hardware using the driver
specific to it. Consider genphy as a fallback which might be good
enough that you can ssh into the board and install the correct
module. You should not really be using it in production.

	Andrew

^ permalink raw reply

* Re: [PATCH v2 04/18] PCI: endpoint: test: Use pci_epc_mem_map/unmap()
From: Niklas Cassel @ 2024-04-05 13:37 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
	Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
	Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
	Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240330041928.1555578-5-dlemoal@kernel.org>

On Sat, Mar 30, 2024 at 01:19:14PM +0900, Damien Le Moal wrote:
> Modify the endpoint test driver to use the functions pci_epc_mem_map()
> and pci_epc_mem_unmap() for the read, write and copy tests. For each
> test case, the transfer (dma or mmio) are executed in a loop to ensure
> that potentially partial mappings returned by pci_epc_mem_map() are
> correctly handled.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---

You probably want to rebase this series on top of:
[1] https://lore.kernel.org/linux-pci/20240314-pci-dbi-rework-v10-0-14a45c5a938e@linaro.org/
[2] https://lore.kernel.org/linux-pci/20240320113157.322695-1-cassel@kernel.org/

As these both modify pci-epf-test.c.

AFAICT both these series [1] (DBI rework v12, not v10) and [2] are fully
reviewed and seem to be ready to go.

They just seem to take a really long time to be picked up.


Kind regards,
Niklas

^ permalink raw reply

* Re: [PATCH v2 06/18] PCI: endpoint: test: Implement link_down event operation
From: Niklas Cassel @ 2024-04-05 13:39 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Damien Le Moal, Lorenzo Pieralisi, Kishon Vijay Abraham I,
	Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
	Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
	Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240403074823.GE25309@thinkpad>

On Wed, Apr 03, 2024 at 01:18:23PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Mar 30, 2024 at 01:19:16PM +0900, Damien Le Moal wrote:
> > Implement the link_down event operation to stop the command execution
> > delayed work when the endpoint controller notifies a link down event.
> > 
> > Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> 
> This patch is already part of another series I posted [1] and under review. So
> this can be dropped.
> 
> - Mani
> 
> [1] https://lore.kernel.org/linux-pci/20240401-pci-epf-rework-v2-9-970dbe90b99d@linaro.org/

Mani, your patch does not use _sync(),
so I don't think that we can simply drop this patch.


Kind regards,
Niklas

> 
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-test.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index ab40c3182677..e6d4e1747c9f 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -824,9 +824,19 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
> >  	return 0;
> >  }
> >  
> > +static int pci_epf_test_link_down(struct pci_epf *epf)
> > +{
> > +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> > +
> > +	cancel_delayed_work_sync(&epf_test->cmd_handler);
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> >  	.core_init = pci_epf_test_core_init,
> >  	.link_up = pci_epf_test_link_up,
> > +	.link_down = pci_epf_test_link_down,
> >  };
> >  
> >  static int pci_epf_test_alloc_space(struct pci_epf *epf)
> > -- 
> > 2.44.0
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

^ permalink raw reply

* Re: [PATCH v8 0/4] ASoc: PCM6240: mixer-test report
From: Mark Brown @ 2024-04-05 13:40 UTC (permalink / raw)
  To: Shenghao Ding
  Cc: linux-kernel, lgirdwood, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, linux-sound, devicetree, perex, tiwai, 13916275206,
	mohit.chawla, soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal,
	aviel, hnagalla, praneeth, Baojun.Xu
In-Reply-To: <20240403003159.389-1-shenghao-ding@ti.com>

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

On Wed, Apr 03, 2024 at 08:31:54AM +0800, Shenghao Ding wrote:
> mixer-test report:
>  root@am335x-evm:/bin# mixer-test
>  TAP version 13
>  # Card 0 - TI BeagleBone Black (TI BeagleBone Black)
>  1..7
>  ok 1 get_value.0.0
>  # 0.0 pcmd3180-i2c-2 Profile id
>  ok 2 name.0.0
>  ok 3 write_default.0.0
>  ok 4 write_valid.0.0
>  ok 5 write_invalid.0.0
>  ok 6 event_missing.0.0
>  ok 7 event_spurious.0.0
>  # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
>  root@am335x-evm:/bin#

None of the additional %s-i2c-%d-dev%d-ch%d-ana-gain type controls
appear to have shown up here - what's the story there?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v2 05/18] PCI: endpoint: test: Synchronously cancel command handler work
From: Niklas Cassel @ 2024-04-05 13:41 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Damien Le Moal, Lorenzo Pieralisi, Kishon Vijay Abraham I,
	Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
	Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
	Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <20240403074702.GD25309@thinkpad>

On Wed, Apr 03, 2024 at 01:17:02PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Mar 30, 2024 at 01:19:15PM +0900, Damien Le Moal wrote:
> > Replace the call to cancel_delayed_work() with a call to
> > cancel_delayed_work_sync() in pci_epf_test_unbind(). This ensures that
> > the command handler is really stopped when proceeding with dma and bar
> > cleanup.
> > 
> > Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> - Mani
> 
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---

Reviewed-by: Niklas Cassel <cassel@kernel.org>

^ permalink raw reply

* Re: [RESEND v7 28/37] dt-bindings: soc: renesas: sh: Add SH7751 based target
From: Geert Uytterhoeven @ 2024-04-05 13:44 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
	Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
	John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
	Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
	Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
	Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
	Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
	Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
	Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
	Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
	Laurent Pinchart, linux-ide, devicetree, linux-kernel,
	linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
	linux-fbdev
In-Reply-To: <3c2937039026fdb827709b2584528aca263f2668.1712207606.git.ysato@users.sourceforge.jp>

Hi Sato-san,

On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Thanks for the update!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/renesas/sh.yaml

> +  compatible:
> +    oneOf:

As adding more SoCs is expected, having oneOf from the start is fine.

> +      - description: SH7751R based platform
> +        items:
> +          - enum:
> +              - renesas,rts7751r2d      # Renesas SH4 2D graphics board
> +              - iodata,landisk          # LANDISK HDL-U
> +              - iodata,usl-5p           # USL-5P
> +          - const: renesas,sh7751r

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v3 2/2] phy: add driver for MediaTek XFI T-PHY
From: Vinod Koul @ 2024-04-05 14:06 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Bc-bocun Chen, Steven Liu, John Crispin, Chunfeng Yun,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Qingfang Deng, SkyLake Huang, Philipp Zabel, linux-arm-kernel,
	linux-mediatek, linux-phy, devicetree, linux-kernel, netdev
In-Reply-To: <ZgXPSrcj0egvzmS6@makrotopia.org>

On 28-03-24, 20:12, Daniel Golle wrote:
> Hi Vinod,
> 
> thank you for taking your time to review my submission!
> 
> On Fri, Mar 29, 2024 at 12:22:47AM +0530, Vinod Koul wrote:
> > On 10-02-24, 02:10, Daniel Golle wrote:
> > > Add driver for MediaTek's XFI T-PHY which can be found in the MT7988
> > 
> > What does XFI mean?
> 
> https://en.wikipedia.org/wiki/XFP_transceiver#XFI
> 
> I chose this name because names of functions dealing with the phy in
> the vendor driver are prefixed "xfi_pextp_".
> The register space used by the phy is called "pextp", which could be
> read as "_P_CI _ex_press _T_-_P_hy", and that is quite misleading as
> this phy isn't used for anything related to PCIe, so I wanted to find
> a better name.
> 
> XFI is still somehow related (as in: you would find the relevant
> places using grep in the vendor driver when looking for that) and
> seemed to at least somehow be aligned with the function of that phy:
> Dealing with (up to) 10 Gbit/s Ethernet SerDes signals.
> 
> MediaTek calls phys with more than one potential use T-PHY or X-PHY:
> The capital letter 'T' graphically connects 3 points, two of them
> being on the upper side representing the internal components and one
> on the lower side representing the single external interface.
> 
> Other vendors (like Marvell) call such things "combo phys".
> 
> Anyway, if anyone has better ideas regarding the naming, now is the
> moment to speak up ;)

This is fine. Combo phys are more common for ones dealing with multiple
components. I am fine either way. But it is good to document why this
was named as such

> 
> 
> > 
> > > SoC. The XFI T-PHY is a 10 Gigabit/s Ethernet SerDes PHY with muxes on
> > > the internal side to be used with either USXGMII PCS or LynxI PCS,
> > > depending on the selected PHY interface mode.
> > > 
> > > The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
> > > PHY_INTERFACE_MODE_* corresponding to the supported modes:
> > > 
> > >  * USXGMII                 \
> > >  * 10GBase-R                }- USXGMII PCS - XGDM  \
> > >  * 5GBase-R                /                        \
> > >                                                      }- Ethernet MAC
> > >  * 2500Base-X              \                        /
> > >  * 1000Base-X               }- LynxI PCS - GDM     /
> > >  * Cisco SGMII (MAC side)  /
> > > 
> > > In order to work-around a performance issue present on the first of
> > > two XFI T-PHYs present in MT7988, special tuning is applied which can be
> > > selected by adding the 'mediatek,usxgmii-performance-errata' property to
> > > the device tree node.
> > > 
> > > There is no documentation for most registers used for the
> > > analog/tuning part, however, most of the registers have been partially
> > > reverse-engineered from MediaTek's SDK implementation (an opaque
> > > sequence of 32-bit register writes) and descriptions for all relevant
> > > digital registers and bits such as resets and muxes have been supplied
> > > by MediaTek.
> > > 
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > ---
> > > v3: no changes
> > > v2:
> > >  * use IO helpers from mtk-io.h instead of rolling my own
> > >  * use devm_clk_bulk_get()
> > >  * yse devm_platform_ioremap_resource()
> > >  * unify name and description everywhere
> > >  * invert bool is_xgmii into bool use_lynxi_pcs and add comments
> > >    describing the meaning of each of the stack variables
> > >  * not much we can do about remaining magic values unless MTK provides
> > >    definitions for them
> > > 
> > > 
> > >  MAINTAINERS                             |   1 +
> > >  drivers/phy/mediatek/Kconfig            |  12 +
> > >  drivers/phy/mediatek/Makefile           |   1 +
> > >  drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 360 ++++++++++++++++++++++++
> > >  4 files changed, 374 insertions(+)
> > >  create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 4be2fd097f261..616b86e3e62fd 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -13776,6 +13776,7 @@ L:	netdev@vger.kernel.org
> > >  S:	Maintained
> > >  F:	drivers/net/phy/mediatek-ge-soc.c
> > >  F:	drivers/net/phy/mediatek-ge.c
> > > +F:	drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > >  
> > >  MEDIATEK I2C CONTROLLER DRIVER
> > >  M:	Qii Wang <qii.wang@mediatek.com>
> > > diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> > > index 3849b7c87d287..117d0e84c7360 100644
> > > --- a/drivers/phy/mediatek/Kconfig
> > > +++ b/drivers/phy/mediatek/Kconfig
> > > @@ -13,6 +13,18 @@ config PHY_MTK_PCIE
> > >  	  callback for PCIe GEN3 port, it supports software efuse
> > >  	  initialization.
> > >  
> > > +config PHY_MTK_XFI_TPHY
> > > +	tristate "MediaTek 10GE SerDes XFI T-PHY driver"
> > > +	depends on ARCH_MEDIATEK || COMPILE_TEST
> > > +	depends on OF && OF_ADDRESS
> > 
> > why both, is OF not enough?
> 
> As we are already also depending on HAS_IOMEM what is left there is
> basically just a !SPARC dependency.
> And that is probably a historic left-over and (according to commit
> 5ab5fc7e35705c from 2010...) should be re-evaluated. I'm happy to drop
> OF_ADDRESS and keep only HAS_IOMEM, and we shall see if any of the
> COMPILE_TESTs actually fails, given that everyone is fine with that.

Yeah HAS_IOMEM would be required for it to get compiled on diff archs. I
think OF should suffice... wdyt?

> 
> > 
> > > +	depends on HAS_IOMEM
> > > +	select GENERIC_PHY
> > > +	help
> > > +	  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
> > > +	  The driver provides access to the Ethernet SerDes T-PHY supporting
> > > +	  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
> > > +	  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
> > > +
> > >  config PHY_MTK_TPHY
> > >  	tristate "MediaTek T-PHY Driver"
> > >  	depends on ARCH_MEDIATEK || COMPILE_TEST
> > > diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> > > index f6e24a47e0815..1b8088df71e84 100644
> > > --- a/drivers/phy/mediatek/Makefile
> > > +++ b/drivers/phy/mediatek/Makefile
> > > @@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)		+= phy-mtk-pcie.o
> > >  obj-$(CONFIG_PHY_MTK_TPHY)		+= phy-mtk-tphy.o
> > >  obj-$(CONFIG_PHY_MTK_UFS)		+= phy-mtk-ufs.o
> > >  obj-$(CONFIG_PHY_MTK_XSPHY)		+= phy-mtk-xsphy.o
> > > +obj-$(CONFIG_PHY_MTK_XFI_TPHY)		+= phy-mtk-xfi-tphy.o
> > >  
> > >  phy-mtk-hdmi-drv-y			:= phy-mtk-hdmi.o
> > >  phy-mtk-hdmi-drv-y			+= phy-mtk-hdmi-mt2701.o
> > > diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > > new file mode 100644
> > > index 0000000000000..551d6cee33f94
> > > --- /dev/null
> > > +++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > > @@ -0,0 +1,360 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/* MediaTek 10GE SerDes XFI T-PHY driver
> > > + *
> > > + * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
> > > + *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
> > > + * based on mtk_usxgmii.c and mtk_sgmii.c found in MediaTek's SDK (GPL-2.0)
> > > + * Copyright (c) 2022 MediaTek Inc.
> > > + * Author: Henry Yen <henry.yen@mediatek.com>
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/device.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/of.h>
> > > +#include <linux/io.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/phy.h>
> > > +#include <linux/phy/phy.h>
> > > +
> > > +#include "phy-mtk-io.h"
> > > +
> > > +#define MTK_XFI_TPHY_NUM_CLOCKS		2
> > > +
> > > +#define REG_DIG_GLB_70			0x0070
> > > +#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)	FIELD_PREP(GENMASK(25, 24), (x))
> > > +#define  XTP_PCS_MODE_MASK		GENMASK(17, 16)
> > > +#define  XTP_PCS_MODE(x)		FIELD_PREP(GENMASK(17, 16), (x))
> > > +#define  XTP_PCS_RST_B			BIT(15)
> > > +#define  XTP_FRC_PCS_RST_B		BIT(14)
> > > +#define  XTP_PCS_PWD_SYNC_MASK		GENMASK(13, 12)
> > > +#define  XTP_PCS_PWD_SYNC(x)		FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
> > > +#define  XTP_PCS_PWD_ASYNC_MASK		GENMASK(11, 10)
> > > +#define  XTP_PCS_PWD_ASYNC(x)		FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
> > > +#define  XTP_FRC_PCS_PWD_ASYNC		BIT(8)
> > > +#define  XTP_PCS_UPDT			BIT(4)
> > > +#define  XTP_PCS_IN_FR_RG		BIT(0)
> > > +
> > > +#define REG_DIG_GLB_F4			0x00f4
> > > +#define  XFI_DPHY_PCS_SEL		BIT(0)
> > > +#define   XFI_DPHY_PCS_SEL_SGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 1)
> > > +#define   XFI_DPHY_PCS_SEL_USXGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 0)
> > > +#define  XFI_DPHY_AD_SGDT_FRC_EN	BIT(5)
> > > +
> > > +#define REG_DIG_LN_TRX_40		0x3040
> > > +#define  XTP_LN_FRC_TX_DATA_EN		BIT(29)
> > > +#define  XTP_LN_TX_DATA_EN		BIT(28)
> > > +
> > > +#define REG_DIG_LN_TRX_B0		0x30b0
> > > +#define  XTP_LN_FRC_TX_MACCK_EN		BIT(5)
> > > +#define  XTP_LN_TX_MACCK_EN		BIT(4)
> > > +
> > > +#define REG_ANA_GLB_D0			0x90d0
> > > +#define  XTP_GLB_USXGMII_SEL_MASK	GENMASK(3, 1)
> > > +#define  XTP_GLB_USXGMII_SEL(x)		FIELD_PREP(GENMASK(3, 1), (x))
> > > +#define  XTP_GLB_USXGMII_EN		BIT(0)
> > > +
> > > +struct mtk_xfi_tphy {
> > > +	void __iomem		*base;
> > > +	struct device		*dev;
> > > +	struct reset_control	*reset;
> > > +	struct clk_bulk_data	clocks[MTK_XFI_TPHY_NUM_CLOCKS];
> > > +	bool			da_war;
> > > +};
> > > +
> > > +static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
> > > +			       phy_interface_t interface)
> > > +{
> > > +	/* Override 10GBase-R tuning value if work-around is selected */
> > > +	bool da_war = (xfi_tphy->da_war && (interface == PHY_INTERFACE_MODE_10GBASER));
> > 
> > why do you need braces around this?
> 
> Just for readability. They can safely be removed.
> 
> > 
> > > +	/* Bools to make setting up values for specific PHY speeds easier */
> > > +	bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
> > > +	bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
> > > +		      interface == PHY_INTERFACE_MODE_SGMII);
> > > +	bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
> > > +		       interface == PHY_INTERFACE_MODE_USXGMII);
> > > +	bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
> > > +	/* Bool to configure input mux to either
> > > +	 *  - USXGMII PCS (64b/66b coding) for 5G/10G
> > > +	 *  - LynxI PCS (8b/10b coding) for 1G/2.5G
> > > +	 */
> > > +	bool use_lynxi_pcs = (is_1g || is_2p5g);
> > 
> > This is quite terrible to read, how about declaring variables first and
> > then doing the initialization?
> 
> Ack.
> 
> > 
> > > +
> > > +	dev_dbg(xfi_tphy->dev, "setting up for mode %s\n", phy_modes(interface));
> > > +
> > > +	/* Setup PLL setting */
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x9024, 0x100000, is_10g ? 0x0 : 0x100000);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x2020, 0x202000, is_5g ? 0x202000 : 0x0);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x2030, 0x500, is_1g ? 0x0 : 0x500);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x2040, 0x340000, is_1g ? 0x200000 : 0x140000);
> > 
> > magic numbers?
> 
> Yes, and not much we can do about them. According to MTK engineers (in
> Cc) they also don't know what those numbers really mean in detail and
> have only been given sequences of magic register writes for each
> interface mode ([1], [2], [3], [4], [5]) by the upstream IP supplier
> of the PHY. I then compared those write sequences with each others,
> and observed the behavior of each register (as in: read their value
> before and after the write operation; all of them read back the value
> written to them) and rewrote the initialization as one function only
> changing the bits actually needed (instead of always writing the complete
> 32-bit value). I've made sure that everything still works and Bc-bocun
> Chen of MediaTek (also in Cc) then helped to label at least some of
> the registers and bits there in as far as they are understood by
> MediaTek.
> 
> [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c#172
> [2]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c#284
> [3]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_usxgmii.c#132
> [4]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_usxgmii.c#246
> [5]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_usxgmii.c#360

Okay lets document the source of magic values so that people may refer
to these

> 
> > 
> > > +
> > > +	/* Setup RXFE BW setting */
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x50f0, 0xc10, is_1g ? 0x410 : is_5g ? 0x800 : 0x400);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
> > > +
> > > +	/* Setup RX CDR setting */
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x5070, 0x670000, is_5g ? 0x620000 : 0x50000);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x5074, 0x180000, is_5g ? 0x180000 : 0x0);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x5078, 0xf000400, is_5g ? 0x8000000 :
> > > +									0x7000400);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x507c, 0x5000500, is_5g ? 0x4000400 :
> > > +									0x1000100);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x5080, 0x1410, is_1g ? 0x400 : is_5g ? 0x1010 : 0x0);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x5084, 0x30300, is_1g ? 0x30300 :
> > > +							      is_5g ? 0x30100 :
> > > +								      0x100);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x5088, 0x60200, is_1g ? 0x20200 :
> > > +							      is_5g ? 0x40000 :
> > > +								      0x20000);
> > > +
> > > +	/* Setting RXFE adaptation range setting */
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x50a8, 0xee0000, is_5g ? 0x800000 :
> > > +								       0x6e0000);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x6004, 0x190000, is_5g ? 0x0 : 0x190000);
> > > +
> > > +	if (is_10g)
> > > +		writel(0x01423342, xfi_tphy->base + 0x00f8);
> > > +	else if (is_5g)
> > > +		writel(0x00a132a1, xfi_tphy->base + 0x00f8);
> > > +	else if (is_2p5g)
> > > +		writel(0x009c329c, xfi_tphy->base + 0x00f8);
> > > +	else
> > > +		writel(0x00fa32fa, xfi_tphy->base + 0x00f8);
> > > +
> > > +	/* Force SGDT_OUT off and select PCS */
> > > +	mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_F4,
> > > +			    XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
> > > +			    XFI_DPHY_AD_SGDT_FRC_EN |
> > > +			    (use_lynxi_pcs ? XFI_DPHY_PCS_SEL_SGMII :
> > > +					     XFI_DPHY_PCS_SEL_USXGMII));
> > > +
> > > +	/* Force GLB_CKDET_OUT */
> > > +	mtk_phy_set_bits(xfi_tphy->base + 0x0030, 0xc00);
> > > +
> > > +	/* Force AEQ on */
> > > +	writel(XTP_PCS_RX_EQ_IN_PROGRESS(2) | XTP_PCS_PWD_SYNC(2) | XTP_PCS_PWD_ASYNC(2),
> > > +	       xfi_tphy->base + REG_DIG_GLB_70);
> > > +
> > > +	usleep_range(1, 5);
> > > +
> > > +	/* Setup TX DA default value */
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x30b0, 0x30, 0x20);
> > > +	writel(0x00008a01, xfi_tphy->base + 0x3028);
> > > +	writel(0x0000a884, xfi_tphy->base + 0x302c);
> > > +	writel(0x00083002, xfi_tphy->base + 0x3024);
> > > +
> > > +	/* Setup RG default value */
> > > +	if (use_lynxi_pcs) {
> > > +		writel(0x00011110, xfi_tphy->base + 0x3010);
> > > +		writel(0x40704000, xfi_tphy->base + 0x3048);
> > > +	} else {
> > > +		writel(0x00022220, xfi_tphy->base + 0x3010);
> > > +		writel(0x0f020a01, xfi_tphy->base + 0x5064);
> > > +		writel(0x06100600, xfi_tphy->base + 0x50b4);
> > > +		if (interface == PHY_INTERFACE_MODE_USXGMII)
> > > +			writel(0x40704000, xfi_tphy->base + 0x3048);
> > > +		else
> > > +			writel(0x47684100, xfi_tphy->base + 0x3048);
> > > +	}
> > > +
> > > +	if (is_1g)
> > > +		writel(0x0000c000, xfi_tphy->base + 0x3064);
> > > +
> > > +	/* Setup RX EQ initial value */
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x3050, 0xa8000000,
> > > +			    (interface != PHY_INTERFACE_MODE_10GBASER) ? 0xa8000000 : 0x0);
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0x3054, 0xaa,
> > > +			    (interface != PHY_INTERFACE_MODE_10GBASER) ? 0xaa : 0x0);
> > > +
> > > +	if (!use_lynxi_pcs)
> > > +		writel(0x00000f00, xfi_tphy->base + 0x306c);
> > > +	else if (is_2p5g)
> > > +		writel(0x22000f00, xfi_tphy->base + 0x306c);
> > > +	else
> > > +		writel(0x20200f00, xfi_tphy->base + 0x306c);
> > > +
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0xa008, 0x10000, da_war ? 0x10000 : 0x0);
> > > +
> > > +	mtk_phy_update_bits(xfi_tphy->base + 0xa060, 0x50000, use_lynxi_pcs ? 0x50000 : 0x40000);
> > > +
> > > +	/* Setup PHYA speed */
> > > +	mtk_phy_update_bits(xfi_tphy->base + REG_ANA_GLB_D0,
> > > +			    XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
> > > +			    is_10g ?  XTP_GLB_USXGMII_SEL(0) :
> > > +			    is_5g ?   XTP_GLB_USXGMII_SEL(1) :
> > > +			    is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
> > > +				      XTP_GLB_USXGMII_SEL(3));
> > > +	mtk_phy_set_bits(xfi_tphy->base + REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
> > > +
> > > +	/* Release reset */
> > > +	mtk_phy_set_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > +			 XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
> > > +	usleep_range(150, 500);
> > > +
> > > +	/* Switch to P0 */
> > > +	mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > +			    XTP_PCS_IN_FR_RG |
> > > +			    XTP_FRC_PCS_PWD_ASYNC |
> > > +			    XTP_PCS_PWD_ASYNC_MASK |
> > > +			    XTP_PCS_PWD_SYNC_MASK |
> > > +			    XTP_PCS_UPDT,
> > > +			    XTP_PCS_IN_FR_RG |
> > > +			    XTP_FRC_PCS_PWD_ASYNC |
> > > +			    XTP_PCS_UPDT);
> > > +	usleep_range(1, 5);
> > > +
> > > +	mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_70, XTP_PCS_UPDT);
> > > +	usleep_range(15, 50);
> > > +
> > > +	if (use_lynxi_pcs) {
> > > +		/* Switch to Gen2 */
> > > +		mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > +				    XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> > > +				    XTP_PCS_MODE(1) | XTP_PCS_UPDT);
> > > +	} else {
> > > +		/* Switch to Gen3 */
> > > +		mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
> > > +				    XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> > > +				    XTP_PCS_MODE(2) | XTP_PCS_UPDT);
> > > +	}
> > > +	usleep_range(1, 5);
> > > +
> > > +	mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_70, XTP_PCS_UPDT);
> > > +
> > > +	usleep_range(100, 500);
> > > +
> > > +	/* Enable MAC CK */
> > > +	mtk_phy_set_bits(xfi_tphy->base + REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
> > > +	mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_F4, XFI_DPHY_AD_SGDT_FRC_EN);
> > > +
> > > +	/* Enable TX data */
> > > +	mtk_phy_set_bits(xfi_tphy->base + REG_DIG_LN_TRX_40,
> > > +			 XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
> > > +	usleep_range(400, 1000);
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode mode, int
> > > +				 submode)
> > > +{
> > > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > +	if (mode != PHY_MODE_ETHERNET)
> > > +		return -EINVAL;
> > > +
> > > +	switch (submode) {
> > > +	case PHY_INTERFACE_MODE_1000BASEX:
> > > +	case PHY_INTERFACE_MODE_2500BASEX:
> > > +	case PHY_INTERFACE_MODE_SGMII:
> > > +	case PHY_INTERFACE_MODE_5GBASER:
> > > +	case PHY_INTERFACE_MODE_10GBASER:
> > > +	case PHY_INTERFACE_MODE_USXGMII:
> > > +		mtk_xfi_tphy_setup(xfi_tphy, submode);
> > > +		return 0;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_reset(struct phy *phy)
> > > +{
> > > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > +	reset_control_assert(xfi_tphy->reset);
> > > +	usleep_range(100, 500);
> > > +	reset_control_deassert(xfi_tphy->reset);
> > > +	usleep_range(1, 10);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_power_on(struct phy *phy)
> > > +{
> > > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > +	return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > > +}
> > > +
> > > +static int mtk_xfi_tphy_power_off(struct phy *phy)
> > > +{
> > > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > > +
> > > +	clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct phy_ops mtk_xfi_tphy_ops = {
> > > +	.power_on	= mtk_xfi_tphy_power_on,
> > > +	.power_off	= mtk_xfi_tphy_power_off,
> > > +	.set_mode	= mtk_xfi_tphy_set_mode,
> > > +	.reset		= mtk_xfi_tphy_reset,
> > > +	.owner		= THIS_MODULE,
> > > +};
> > > +
> > > +static int mtk_xfi_tphy_probe(struct platform_device *pdev)
> > > +{
> > > +	struct device_node *np = pdev->dev.of_node;
> > > +	struct phy_provider *phy_provider;
> > > +	struct mtk_xfi_tphy *xfi_tphy;
> > > +	struct phy *phy;
> > > +	int ret;
> > > +
> > > +	if (!np)
> > > +		return -ENODEV;
> > > +
> > > +	xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
> > > +	if (!xfi_tphy)
> > > +		return -ENOMEM;
> > > +
> > > +	xfi_tphy->base = devm_platform_ioremap_resource(pdev, 0);
> > > +	if (IS_ERR(xfi_tphy->base))
> > > +		return PTR_ERR(xfi_tphy->base);
> > > +
> > > +	xfi_tphy->dev = &pdev->dev;
> > > +	xfi_tphy->clocks[0].id = "topxtal";
> > > +	xfi_tphy->clocks[1].id = "xfipll";
> > > +	ret = devm_clk_bulk_get(&pdev->dev, MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > > +	if (IS_ERR(xfi_tphy->reset))
> > > +		return PTR_ERR(xfi_tphy->reset);
> > > +
> > > +	xfi_tphy->da_war = of_property_read_bool(np, "mediatek,usxgmii-performance-errata");
> > > +
> > > +	phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
> > > +	if (IS_ERR(phy))
> > > +		return PTR_ERR(phy);
> > > +
> > > +	phy_set_drvdata(phy, xfi_tphy);
> > > +	phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
> > > +
> > > +	return PTR_ERR_OR_ZERO(phy_provider);
> > > +}
> > > +
> > > +static const struct of_device_id mtk_xfi_tphy_match[] = {
> > > +	{ .compatible = "mediatek,mt7988-xfi-tphy", },
> > > +	{ /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
> > > +
> > > +static struct platform_driver mtk_xfi_tphy_driver = {
> > > +	.probe = mtk_xfi_tphy_probe,
> > > +	.driver = {
> > > +		.name = "mtk-xfi-tphy",
> > > +		.of_match_table = mtk_xfi_tphy_match,
> > > +	},
> > > +};
> > > +module_platform_driver(mtk_xfi_tphy_driver);
> > > +
> > > +MODULE_DESCRIPTION("MediaTek 10GE SerDes XFI T-PHY driver");
> > > +MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
> > > +MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
> > > +MODULE_LICENSE("GPL");
> > > -- 
> > > 2.43.0
> > 
> > -- 
> > ~Vinod

-- 
~Vinod

^ permalink raw reply

* [PATCH 0/4] Introduce msm8916 based Motorola devices
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin,
	Ruby Iris Juric, Stephan Gerhold, Wiktor Strzębała,
	Valérie Roux, Martijn Braam

This series introduces a set of msm8916 bsed Motorola devices:

- Moto G4 Play (harpia)
- Moto G 2015 (osprey)
- Moto E 2015 LTE (surnia)

The submission brings up the following features:

- eMMC and SD;
- Buttons;
- Touchscreen;
- USB;
- Fuel Gauge;
- Sound;
- Accelerometer (harpia only).

Since the devices share a lot of similarities, the common parts of the
DT are separated out into a dedicated dtsi, introduced with the first
device.

Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
Martijn Braam (1):
      arm64: dts: qcom: Add Motorola Moto G 2015 (osprey)

Nikita Travkin (1):
      dt-bindings: arm: qcom: Add msm8916 based Motorola devices

Ruby Iris Juric (1):
      arm64: dts: qcom: Add device tree for Motorola Moto G4 Play (harpia)

Wiktor Strzębała (1):
      arm64: dts: qcom: Add Motorola Moto E 2015 LTE (surnia)

 Documentation/devicetree/bindings/arm/qcom.yaml    |   3 +
 arch/arm64/boot/dts/qcom/Makefile                  |   3 +
 .../boot/dts/qcom/msm8916-motorola-common.dtsi     | 161 +++++++++++++++++++++
 .../boot/dts/qcom/msm8916-motorola-harpia.dts      | 147 +++++++++++++++++++
 .../boot/dts/qcom/msm8916-motorola-osprey.dts      | 105 ++++++++++++++
 .../boot/dts/qcom/msm8916-motorola-surnia.dts      |  83 +++++++++++
 6 files changed, 502 insertions(+)
---
base-commit: 29493ca7d6b1d3fdc224467c422ac9bdf6d7a252
change-id: 20240405-msm8916-moto-init-640862b8f57c

Best regards,
-- 
Nikita Travkin <nikita@trvn.ru>


^ permalink raw reply

* [PATCH 1/4] dt-bindings: arm: qcom: Add msm8916 based Motorola devices
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin
In-Reply-To: <20240405-msm8916-moto-init-v1-0-502b58176d34@trvn.ru>

Add compatible values for the msm8916 based Motorola smartphones.

Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 1646e5bd23d8..29ff7c833909 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -197,6 +197,9 @@ properties:
               - huawei,g7
               - longcheer,l8910
               - longcheer,l8150
+              - motorola,harpia
+              - motorola,osprey
+              - motorola,surnia
               - qcom,msm8916-mtp
               - samsung,a3u-eur
               - samsung,a5u-eur

-- 
2.44.0


^ permalink raw reply related

* [PATCH 2/4] arm64: dts: qcom: Add device tree for Motorola Moto G4 Play (harpia)
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin,
	Ruby Iris Juric, Stephan Gerhold
In-Reply-To: <20240405-msm8916-moto-init-v1-0-502b58176d34@trvn.ru>

From: Ruby Iris Juric <ruby@srxl.me>

Motorola Moto G4 Play is an msm8916 based smartphone.

Supported features:

- eMMC and SD;
- Buttons;
- Touchscreen;
- USB;
- Fuel Gauge;
- Sound;
- Accelerometer.

msm8916 Moto devices share significant portion of the design so the
common parts are separated into a common dtsi.

Signed-off-by: Ruby Iris Juric <ruby@srxl.me>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Nikita: Split up to common dtsi]
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 arch/arm64/boot/dts/qcom/Makefile                  |   1 +
 .../boot/dts/qcom/msm8916-motorola-common.dtsi     | 161 +++++++++++++++++++++
 .../boot/dts/qcom/msm8916-motorola-harpia.dts      | 147 +++++++++++++++++++
 3 files changed, 309 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 7d40ec5e7d21..8d3fc7cb7a4d 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -31,6 +31,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-gplus-fl8005a.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-huawei-g7.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-longcheer-l8150.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-longcheer-l8910.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-motorola-harpia.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-a3u-eur.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-a5u-eur.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-motorola-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-motorola-common.dtsi
new file mode 100644
index 000000000000..6a27d0ecd2ad
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-motorola-common.dtsi
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	aliases {
+		mmc0 = &sdhc_1; /* eMMC */
+		mmc1 = &sdhc_2; /* SD card */
+		serial0 = &blsp_uart1;
+	};
+
+	chosen {
+		stdout-path = "serial0";
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&gpio_keys_default>;
+		pinctrl-names = "default";
+
+		label = "GPIO Buttons";
+
+		volume-up-button {
+			label = "Volume Up";
+			gpios = <&tlmm 107 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_VOLUMEUP>;
+			debounce-interval = <15>;
+		};
+	};
+
+	usb_id: usb-id {
+		compatible = "linux,extcon-usb-gpio";
+		id-gpios = <&tlmm 91 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&usb_id_default>;
+		pinctrl-1 = <&usb_id_sleep>;
+		pinctrl-names = "default", "sleep";
+	};
+};
+
+&blsp_i2c2 {
+	status = "okay";
+
+	touchscreen: touchscreen@20 {
+		compatible = "syna,rmi4-i2c";
+		reg = <0x20>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		vio-supply = <&pm8916_l6>;
+
+		syna,startup-delay-ms = <100>;
+
+		rmi4-f01@1 {
+			reg = <1>;
+			syna,nosleep-mode = <1>; /* Allow sleeping */
+		};
+
+		rmi4-f11@11 {
+			reg = <11>;
+			syna,sensor-type = <1>; /* Touchscreen */
+		};
+	};
+};
+
+&blsp_uart1 {
+	status = "okay";
+};
+
+&mpss_mem {
+	reg = <0x0 0x86800000 0x0 0x5500000>;
+};
+
+&pm8916_resin {
+	linux,code = <KEY_VOLUMEDOWN>;
+	status = "okay";
+};
+
+&pm8916_rpm_regulators {
+	pm8916_l16: l16 {
+		regulator-min-microvolt = <3100000>;
+		regulator-max-microvolt = <3300000>;
+	};
+};
+
+&pm8916_vib {
+	status = "okay";
+};
+
+&sdhc_1 {
+	status = "okay";
+};
+
+&sdhc_2 {
+	status = "okay";
+};
+
+&usb {
+	extcon = <&usb_id>, <&usb_id>;
+	status = "okay";
+};
+
+&usb_hs_phy {
+	extcon = <&usb_id>;
+};
+
+&venus {
+	status = "okay";
+};
+
+&venus_mem {
+	status = "okay";
+};
+
+&wcnss {
+	status = "okay";
+};
+
+&wcnss_iris {
+	compatible = "qcom,wcn3620";
+};
+
+&wcnss_mem {
+	status = "okay";
+};
+
+/* CTS/RTX are not used */
+&blsp_uart1_default {
+	pins = "gpio0", "gpio1";
+};
+&blsp_uart1_sleep {
+	pins = "gpio0", "gpio1";
+};
+
+&tlmm {
+	gpio_keys_default: gpio-keys-default-state {
+		pins = "gpio107";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
+	usb_id_default: usb-id-default-state {
+		pins = "gpio91";
+		function = "gpio";
+		drive-strength = <8>;
+		bias-pull-up;
+	};
+
+	usb_id_sleep: usb-id-sleep-state {
+		pins = "gpio91";
+		function = "gpio";
+		drive-strength = <8>;
+		bias-disable;
+	};
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916-motorola-harpia.dts b/arch/arm64/boot/dts/qcom/msm8916-motorola-harpia.dts
new file mode 100644
index 000000000000..8380451ebbf6
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-motorola-harpia.dts
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-motorola-common.dtsi"
+
+/ {
+	model = "Motorola Moto G4 Play";
+	compatible = "motorola,harpia", "qcom,msm8916";
+	chassis-type = "handset";
+};
+
+&blsp_i2c1 {
+	status = "okay";
+
+	battery@36 {
+		compatible = "maxim,max17050";
+		reg = <0x36>;
+
+		interrupts-extended = <&tlmm 62 IRQ_TYPE_EDGE_FALLING>;
+
+		pinctrl-0 = <&battery_alert_default>;
+		pinctrl-names = "default";
+
+		maxim,rsns-microohm = <10000>;
+		maxim,over-heat-temp = <600>;
+		maxim,cold-temp = <(-200)>;
+		maxim,dead-volt = <3200>;
+		maxim,over-volt = <4500>;
+	};
+
+	/* charger@6b */
+};
+
+&blsp_i2c4 {
+	status = "okay";
+
+	accelerometer@19 {
+		compatible = "bosch,bma253";
+		reg = <0x19>;
+
+		interrupts-extended = <&tlmm 115 IRQ_TYPE_EDGE_RISING>,
+				      <&tlmm 119 IRQ_TYPE_EDGE_RISING>;
+
+		vdd-supply = <&pm8916_l17>;
+		vddio-supply = <&pm8916_l6>;
+
+		mount-matrix = "1",  "0", "0",
+			       "0", "-1", "0",
+			       "0",  "0", "1";
+
+		pinctrl-0 = <&accel_int_default>;
+		pinctrl-names = "default";
+	};
+
+	/* proximity@49 */
+};
+
+&pm8916_codec {
+	qcom,micbias-lvl = <2800>;
+	qcom,mbhc-vthreshold-low = <75 150 237 450 500>;
+	qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
+	qcom,micbias1-ext-cap;
+};
+
+&pm8916_rpm_regulators {
+	pm8916_l17: l17 {
+		regulator-min-microvolt = <2850000>;
+		regulator-max-microvolt = <2850000>;
+	};
+};
+
+&sdhc_2 {
+	pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
+	pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
+	pinctrl-names = "default", "sleep";
+
+	cd-gpios = <&tlmm 118 GPIO_ACTIVE_LOW>;
+};
+
+&sound {
+	audio-routing =
+		"AMIC1", "MIC BIAS External1",
+		"AMIC2", "MIC BIAS Internal2",
+		"AMIC3", "MIC BIAS External1";
+
+	pinctrl-0 = <&cdc_pdm_default &headset_switch_supply_en
+		     &headset_switch_in>;
+	pinctrl-1 = <&cdc_pdm_sleep &headset_switch_supply_en
+		     &headset_switch_in>;
+	pinctrl-names = "default", "sleep";
+};
+
+&touchscreen {
+	interrupts-extended = <&tlmm 13 IRQ_TYPE_EDGE_FALLING>;
+
+	vdd-supply = <&pm8916_l16>;
+
+	pinctrl-0 = <&ts_int_default>;
+	pinctrl-names = "default";
+};
+
+&tlmm {
+	accel_int_default: accel-int-default-state {
+		pins = "gpio115", "gpio119";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	battery_alert_default: battery-alert-default-state {
+		pins = "gpio62";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
+	headset_switch_in: headset-switch-in-state {
+		pins = "gpio112";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-low;
+	};
+
+	headset_switch_supply_en: headset-switch-supply-en-state {
+		pins = "gpio111";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-high;
+	};
+
+	sdc2_cd_default: sdc2-cd-default-state {
+		pins = "gpio118";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	ts_int_default: ts-int-default-state {
+		pins = "gpio13";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+};

-- 
2.44.0


^ permalink raw reply related

* [PATCH 3/4] arm64: dts: qcom: Add Motorola Moto E 2015 LTE (surnia)
From: Nikita Travkin @ 2024-04-05 14:06 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Nikita Travkin,
	Wiktor Strzębała, Valérie Roux, Stephan Gerhold
In-Reply-To: <20240405-msm8916-moto-init-v1-0-502b58176d34@trvn.ru>

From: Wiktor Strzębała <wiktorek140@gmail.com>

Motorola Moto E 2015 LTE is an msm8916 based smartphone.

Supported features:

- eMMC and SD;
- Buttons;
- Touchscreen;
- USB;
- Fuel Gauge;
- Sound.

Signed-off-by: Wiktor Strzębała <wiktorek140@gmail.com>
[Valérie: Sound and modem]
Co-developed-by: Valérie Roux <undev@unixgirl.xyz>
Signed-off-by: Valérie Roux <undev@unixgirl.xyz>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Nikita: Use common dtsi]
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 arch/arm64/boot/dts/qcom/Makefile                  |  1 +
 .../boot/dts/qcom/msm8916-motorola-surnia.dts      | 83 ++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 8d3fc7cb7a4d..acf3ee316fba 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -32,6 +32,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-huawei-g7.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-longcheer-l8150.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-longcheer-l8910.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-motorola-harpia.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-motorola-surnia.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-a3u-eur.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-a5u-eur.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-motorola-surnia.dts b/arch/arm64/boot/dts/qcom/msm8916-motorola-surnia.dts
new file mode 100644
index 000000000000..eecf78ba45bb
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-motorola-surnia.dts
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-motorola-common.dtsi"
+
+/ {
+	model = "Motorola Moto E 2015 LTE";
+	compatible = "motorola,surnia", "qcom,msm8916";
+	chassis-type = "handset";
+};
+
+&blsp_i2c4 {
+	status = "okay";
+
+	battery@36 {
+		compatible = "maxim,max17050";
+		reg = <0x36>;
+
+		interrupts-extended = <&tlmm 12 IRQ_TYPE_EDGE_FALLING>;
+
+		pinctrl-0 = <&battery_alert_default>;
+		pinctrl-names = "default";
+
+		maxim,rsns-microohm = <10000>;
+		maxim,over-heat-temp = <600>;
+		maxim,cold-temp = <(-200)>;
+		maxim,dead-volt = <3200>;
+		maxim,over-volt = <4500>;
+
+	};
+};
+
+&pm8916_codec {
+	qcom,micbias1-ext-cap;
+	qcom,micbias2-ext-cap;
+};
+
+&sdhc_2 {
+	pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
+	pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
+	pinctrl-names = "default", "sleep";
+
+	cd-gpios = <&tlmm 25 GPIO_ACTIVE_LOW>;
+};
+
+&sound {
+	audio-routing =
+		"AMIC1", "MIC BIAS External1",
+		"AMIC3", "MIC BIAS External1";
+};
+
+&touchscreen {
+	interrupts-extended = <&tlmm 21 IRQ_TYPE_EDGE_FALLING>;
+
+	vdd-supply = <&pm8916_l16>;
+
+	pinctrl-0 = <&ts_int_default>;
+	pinctrl-names = "default";
+};
+
+&tlmm {
+	battery_alert_default: battery-alert-default-state {
+		pins = "gpio12";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
+	sdc2_cd_default: sdc2-cd-default-state {
+		pins = "gpio25";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	ts_int_default: ts-int-default-state {
+		pins = "gpio21";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+};

-- 
2.44.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox