On 08/03/2023 21:09, Dipen Patel wrote: > On 3/8/23 11:05 AM, Krzysztof Kozlowski wrote: >> On 08/03/2023 19:45, Dipen Patel wrote: >>> On 2/16/23 6:17 AM, Krzysztof Kozlowski wrote: >>>> On 14/02/2023 12:55, Dipen Patel wrote: >>>>> Added timestamp provider support for the Tegra234 in devicetree >>>>> bindings. >>>> >>>> 1. Your commit does much more. You need to explain it why you drop some >>>> property. >>> ACK, will address it next patch >>>> >>>> 2. Bindings go before its usage (in the patchset). >>> Ack... >>>> >>>> 3. Please use scripts/get_maintainers.pl to get a list of necessary >>>> people and lists to CC. It might happen, that command when run on an >>>> older kernel, gives you outdated entries. Therefore please be sure you >>>> base your patches on recent Linux kernel. >>> It is based on recent linux at the time patch series was sent... >> >> That's good but then why you do not use scripts/get_maintainers.pl? The >> hint about recent kernel was just a hint... Just do not invent addresses >> by yourself and use the tool to get them right. >> > I will take a note for the next patch series to add any missing people. The current > list of people/group is what historically helped review this new timestamp/hte subsystem. > >> (...) >> >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - nvidia,tegra194-gte-aon >>>> >>>> This is an ABI break. Does your driver handle it? >>> yes, handling patch is part of this patch series. >> >> Can you point me to the code which does it? I see "return -ENODEV;", so >> I think you do not handle ABI break. I could miss something but since >> you disagree with me, please at least bring some arguments... > Refer to patch https://patchwork.kernel.org/project/timestamp/patch/20230214115553.10416-3-dipenp@xxxxxxxxxx/ > which has compatible properties added and also code changes to reflect addition/deletion of some > properties. I referred to the code which breaks the ABI. > > I am not sure I have understood about ABI break comment. How else one should handle if > there is no related gpio controller property found? In a way it does not break existing users? There are many ways to handle it, but I don't know your code to point you. > I am assuming you are referring to the > below code from the patch 2 (link above) when you said "return -ENODEV". Your bindings patch points to ABI break without any explanation/justification. Then your code #2 patch actually breaks it, also without any justification. Best regards, Krzysztof
