On 06/12/2018 03:54 PM, Rob Herring wrote: > On Thu, Jun 7, 2018 at 12:53 PM, Scott Branden > <scott.branden@xxxxxxxxxxxx> wrote: >> Hi Rob, >> >> Could you please kindly comment on change below. >> >> It allows board variants to be added easily via a simple define for >> different number of SATA ports. >> >> >> >> On 18-06-04 09:22 AM, Florian Fainelli wrote: >>> >>> On 05/18/2018 11:34 AM, Scott Branden wrote: >>>> >>>> Move remaining sata configuration to stingray-sata.dtsi and enable >>>> ports based on NUM_SATA defined. >>>> Now, all that needs to be done is define NUM_SATA per board. >>> >>> Rob could you review this and let us know if this approach is okay or >>> not? Thank you! >>> >>>> Signed-off-by: Scott Branden <scott.branden@xxxxxxxxxxxx> >>>> --- > >>>> diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray-sata.dtsi >>>> b/arch/arm64/boot/dts/broadcom/stingray/stingray-sata.dtsi >>>> index 8c68e0c..7f6d176 100644 >>>> --- a/arch/arm64/boot/dts/broadcom/stingray/stingray-sata.dtsi >>>> +++ b/arch/arm64/boot/dts/broadcom/stingray/stingray-sata.dtsi >>>> @@ -43,7 +43,11 @@ >>>> interrupts = <GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH>; >>>> #address-cells = <1>; >>>> #size-cells = <0>; >>>> +#if (NUM_SATA > 0) >>>> + status = "okay"; >>>> +#else >>>> status = "disabled"; >>>> +#endif > > This only works if ports are contiguously enabled (0-N). You might not > care, but it is not a pattern that works in general. And I'm not a fan > of C preprocessing in DT files in general beyond just defines for > single numbers. Should we interpret this as a formal NAK? -- Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html