| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <D9RNO4O3YN4X.1G09DC9I8JGJF@cknow.org>
Date: Fri, 09 May 2025 15:14:31 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Sebastian Reichel" <sebastian.reichel@...labora.com>,
Heiko Stübner <heiko@...ech.de>
Cc: "Rob Herring" <robh@...nel.org>, "Krzysztof Kozlowski"
<krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>,
<devicetree@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
<linux-rockchip@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<kernel@...labora.com>
Subject: Re: [PATCH v2 2/5] arm64: dts: rockchip: move rock 5b to include
file
Hi,
On Fri May 9, 2025 at 3:08 PM CEST, Sebastian Reichel wrote:
> On Fri, May 09, 2025 at 02:54:00PM +0200, Heiko Stübner wrote:
>> Am Freitag, 9. Mai 2025, 14:44:57 Mitteleuropäische Sommerzeit schrieb Diederik de Haas:
>> > On Thu May 8, 2025 at 7:48 PM CEST, Sebastian Reichel wrote:
>> > > Radxa released some more boards, which are based on the original
>> > > Rock 5B. Move its board description into an include file to avoid
>> > > unnecessary duplication.
>> >
>> > Aren't you moving it *out of* an/the include file?
>> > If so, the patch Subject and the above line should be updated so that
>> > they correctly reflect what is changed in this patch.
>> >
>> > The above text is correct (and the same ...) as patch 1, but in this
>> > patch you move things out of the dtsi which are unique per board.
>> >
>> > > NOTE: this should be merged with the previous commit to ensure
>> > > bisectability. The rename happens in a separete commit during
>> > > development because git does not properly detect the rename when
>> > > the original filename is reused in the same commit. This means
>> > >
>> > > 1. it's a lot harder to review the changes
>> > > 2. it's a lot harder to rebase the patch series
>> >
>> > Or did I fall prey to the exact thing you described here?
>>
>> I think Sebastian's idea is, that I squash both patches when applying.
>> This split makes it easy(er) to review because patch1 is just a rename.
>>
>> And merging them when applying then makes it again not break bisectability.
>
> Correct. This is a lot easier to review than what git generates when
> having these two patches squashed together, which is a huge diff of
> all 1000+ lines in the file (I tried really hard to convince it that
> this is mostly a rename with --find-renames and --find-copies). You
> can see this kind of mess in patch 2 of the ROCK 5T series that
> Nicolas just send (I will comment on that and suggest to do the same
> thing I did to ease review. In his case it should even be possible
> to do it in a bisectable way without needing a squash :)).
Thank you both for the clarification :-)
Cheers,
Diederik
>> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.com>
>> > > ---
>> > > arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 52 ++++++++++++++++++++++++
>> > > arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi | 40 ------------------
>> > > 2 files changed, 52 insertions(+), 40 deletions(-)
>> > >
>> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
>> > > new file mode 100644
>> > > index 0000000000000000000000000000000000000000..9407a7c9910ada1f6c803d2e15785a9cbd9bd655
>> > > --- /dev/null
>> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
>> > > @@ -0,0 +1,52 @@
>> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> > > +
>> > > +/dts-v1/;
>> > > +
>> > > +#include "rk3588-rock-5b.dtsi"
>> > > +
>> > > +/ {
>> > > + model = "Radxa ROCK 5B";
>> > > + compatible = "radxa,rock-5b", "rockchip,rk3588";
>> > > +};
>> > > +
>> > > +&sdio {
>> > > + max-frequency = <200000000>;
>> > > + no-sd;
>> > > + no-mmc;
>> > > + non-removable;
>> > > + bus-width = <4>;
>> > > + cap-sdio-irq;
>> > > + disable-wp;
>> > > + keep-power-in-suspend;
>> > > + wakeup-source;
>> > > + sd-uhs-sdr12;
>> > > + sd-uhs-sdr25;
>> > > + sd-uhs-sdr50;
>> > > + sd-uhs-sdr104;
>> > > + vmmc-supply = <&vcc3v3_pcie2x1l0>;
>> > > + vqmmc-supply = <&vcc_1v8_s3>;
>> > > + pinctrl-names = "default";
>> > > + pinctrl-0 = <&sdiom0_pins>;
>> > > + status = "okay";
>> > > +};
>> > > +
>> > > +&uart6 {
>> > > + pinctrl-names = "default";
>> > > + pinctrl-0 = <&uart6m1_xfer &uart6m1_ctsn &uart6m1_rtsn>;
>> > > + status = "okay";
>> > > +};
>> > > +
>> > > +&pinctrl {
>> > > + usb {
>> > > + vcc5v0_host_en: vcc5v0-host-en {
>> > > + rockchip,pins = <4 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
>> > > + };
>> > > + };
>> > > +};
>> > > +
>> > > +&vcc5v0_host {
>> > > + enable-active-high;
>> > > + gpio = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
>> > > + pinctrl-names = "default";
>> > > + pinctrl-0 = <&vcc5v0_host_en>;
>> > > +};
>> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
>> > > index 17f4fd054cd3d1c4e23ccfe014a9c4b9d7ad1a06..6052787d2560978d2bae6cfbeea5fc1d419d583a 100644
>> > > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
>> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
>> > > @@ -8,9 +8,6 @@
>> > > #include "rk3588.dtsi"
>> > >
>> > > / {
>> > > - model = "Radxa ROCK 5B";
>> > > - compatible = "radxa,rock-5b", "rockchip,rk3588";
>> > > -
>> > > aliases {
>> > > mmc0 = &sdhci;
>> > > mmc1 = &sdmmc;
>> > > @@ -139,10 +136,6 @@ vcc5v0_host: regulator-vcc5v0-host {
>> > > regulator-always-on;
>> > > regulator-min-microvolt = <5000000>;
>> > > regulator-max-microvolt = <5000000>;
>> > > - enable-active-high;
>> > > - gpio = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
>> > > - pinctrl-names = "default";
>> > > - pinctrl-0 = <&vcc5v0_host_en>;
>> > > vin-supply = <&vcc5v0_sys>;
>> > > };
>> > >
>> > > @@ -488,12 +481,6 @@ pcie3_vcc3v3_en: pcie3-vcc3v3-en {
>> > > rockchip,pins = <1 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
>> > > };
>> > > };
>> > > -
>> > > - usb {
>> > > - vcc5v0_host_en: vcc5v0-host-en {
>> > > - rockchip,pins = <4 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
>> > > - };
>> > > - };
>> > > };
>> > >
>> > > &pwm1 {
>> > > @@ -530,27 +517,6 @@ &sdmmc {
>> > > status = "okay";
>> > > };
>> > >
>> > > -&sdio {
>> > > - max-frequency = <200000000>;
>> > > - no-sd;
>> > > - no-mmc;
>> > > - non-removable;
>> > > - bus-width = <4>;
>> > > - cap-sdio-irq;
>> > > - disable-wp;
>> > > - keep-power-in-suspend;
>> > > - wakeup-source;
>> > > - sd-uhs-sdr12;
>> > > - sd-uhs-sdr25;
>> > > - sd-uhs-sdr50;
>> > > - sd-uhs-sdr104;
>> > > - vmmc-supply = <&vcc3v3_pcie2x1l0>;
>> > > - vqmmc-supply = <&vcc_1v8_s3>;
>> > > - pinctrl-names = "default";
>> > > - pinctrl-0 = <&sdiom0_pins>;
>> > > - status = "okay";
>> > > -};
>> > > -
>> > > &sfc {
>> > > pinctrl-names = "default";
>> > > pinctrl-0 = <&fspim2_pins>;
>> > > @@ -566,12 +532,6 @@ flash@0 {
>> > > };
>> > > };
>> > >
>> > > -&uart6 {
>> > > - pinctrl-names = "default";
>> > > - pinctrl-0 = <&uart6m1_xfer &uart6m1_ctsn &uart6m1_rtsn>;
>> > > - status = "okay";
>> > > -};
>> > > -
>> > > &spi2 {
>> > > status = "okay";
>> > > assigned-clocks = <&cru CLK_SPI2>;
>> >
>> >
>>
>>
>>
>>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists