On Tue, 31 Dec 2019 at 11:23, Artur Świgoń [off-list ref] wrote:

quoted

The order of patches should reflect first of all real dependency.
Whether it compiles, works at all and does not break anything.  Logical
dependency of "when the feature will start working" is
irrelevant to DTS because DTS goes in separate way and driver is
independent of it.

The order of patches does indeed reflect real dependency. I can also reorder
them (preserving the dependencies) so that DTS patches go first in the series
if this is the more preferred way.

It looks wrong then. Driver should not depend on DTS. I cannot find
the patch changing bindings (should be first in patchset) which could
also point to this problem.

It seems you added requirement for interconnect properties while it
should be rather optional.

quoted

quoted

I still think the order of these patches is the most logical one for someone
reading this RFC as a whole.

I am sorry but it brings only confusion. DTS is orthogonal of the
driver code. You could even post the patchset without DTS (although then
it would raise questions where is the user of it, but still, you
could).

Further, DTS describes also hardware so you could send certain DTS
patches without driver implementation to describe the hardware.

Driver code and DTS are kind of different worlds so mixing them up for
logical review does not really make any sense.

Not mentioning it is different than most of other patches on mailing
lists.

BTW, it is the same as bindings which should (almost) always go first as
separate patches.

Thanks for elaborating on this, I appreciate it.
Regarding your original concern, patches 04 & 06 are separate for several
reasons, one of which is that they are related to two different drivers
(exynos-bus vs. exynos-mixer).

It's okay then (for them to be split).

quoted

quoted

quoted

In certain cases dependency on DTS changes is ok:
1. Cleaning up deprecated properties,
2. Ignoring the backward compatibility for e.g. new platforms.

None of these are applicable here.

You need to rework it, put DTS changes at the end. This clearly shows
that there is no wrong dependency.

quoted

quoted

Adjust the title to match the contents - you are not adding bindings but
properties to bus nodes. Also the prefix is ARM: (look at recent
commits).

OK.

quoted

quoted

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 4ce3d77a6704..d9d70eacfcaf 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi

@@ -90,6 +90,7 @@
 &bus_dmc {
    exynos,ppmu-device = <&ppmu_dmc0_3>, <&ppmu_dmc1_3>;
    vdd-supply = <&buck1_reg>;
+   #interconnect-cells = <0>;

This does not look like property of Odroid but Exynos4412 or Exynos4.

Strangely enough, this file is where the 'exynos,parent-bus' (aka. 'devfreq')
properties are located (and everything in this RFC concerns devfreq).

I cannot find exynos,parent-bus in exynos4412-odroid-common.dtsi. Can
you elaborate?

Currently a name change is being made: 'devfreq' -> 'exynos,parent-bus'
https://patchwork.kernel.org/patch/11304549/
(a dependency of this RFC; also available in devfreq-testing branch)

I see. That property also does not look like board (Odroid) specific so
it should be moved to Exynos4412 DTSI.

Makes sense to me. Just from looking at the patch I referenced above, there is
a significant level of code duplication between
* arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi
* arch/arm/boot/dts/exynos4412-midas.dtsi
* arch/arm/boot/dts/exynos4412-odroid-common.dtsi
with relation to the devfreq*/exynos,* properties.

If you have in mind all the nodes with "status=okay", it's fine to
duplicate them.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel