Re: [PATCH v2 3/5] dt/bindings: ti,edma: Remove redundant properties from documentation

From: Peter Ujfalusi
Date: Thu May 15 2014 - 04:19:10 EST


On 05/15/2014 11:01 AM, Arnd Bergmann wrote:
> On Tuesday 13 May 2014 13:30:30 Peter Ujfalusi wrote:
>> From CCCFG register of eDMA3 we can get all the needed information for the
>> driver about the IP:
>> Number of channels: NUM_DMACH
>> Number of regions: NUM_REGN
>> Number of slots (PaRAM sets): NUM_PAENTRY
>> Number of TC/EQ: NUM_EVQUE
>>
>> The ti,edma-regions; ti,edma-slots and dma-channels in DT are
>> redundant since the very same information can be obtained from the HW.
>> The mentioned properties can be removed from the binding document.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
>
> I wonder if we should keep them listed as "optional" properties so you
> can have a dtb file that still works with older kernels which need them.
>
> What you do is an incompatible change to the binding, which we shouldn't
> do lightly. Any new dts files don't need this information of course, but
> as a general rule, I'd rather keep things like this around unless we
> already have to enforce an ABI break that is well documented.

We could keep them as optional, or to be precise: ignored properties since we
are not going to even look at them in the future.
But I do not really see the reason to do so. With this change new kernels will
boot with old DTB - if it can not be changed which I have not seen yet. Sure
old kernel would not boot with this change, but why would you downgrade the
kernel and update the DTB on a board?
Bisecting is not a good reason since the bug you might be hunting for could be
in the DTS or in the kernel code so you need to update both of them to be sure
you reach the commit you are looking for.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/