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 am not sure I have understood about ABI break comment. How else one should handle if there is no related gpio controller property found? I am assuming you are referring to the below code from the patch 2 (link above) when you said "return -ENODEV". - hte_dev->c = gpiochip_find("tegra194-gpio-aon", - tegra_get_gpiochip_from_name); + gpio_ctrl = of_parse_phandle(dev->of_node, + "nvidia,gpio-controller", 0); + if (!gpio_ctrl) { + dev_err(dev, "gpio controller node not found\n"); + return -ENODEV; + } + + hte_dev->c = gpiochip_find(gpio_ctrl, + tegra_get_gpiochip_from_of_node) > > > Best regards, > Krzysztof >
