Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Rob, On 17-06-29 03:40 PM, Rob Herring wrote: On Tue, Jun 27, 2017 at 12:10 AM, Scott Brandenwrote: Hi Rob/Florian, Thanks for input but still don't see any need for SoC specific compatible stings. IP revision specific yes. On 17-06-22 06:04 PM, Florian Fainelli wrote: On 06/22/2017 05:42 PM, Scott Branden wrote: On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden wrote: Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. I don't understand what you mean by fallback compatible then. Let's say I have 3 SoCs that each contain the same ip block. You want us to add a fallback compatibility per SoC, is that correct? I think Rob meant a fallback compatibility for the particular block. E.g: brcm,iproc-ptp is the fallback compatible string, but in your SoC-specific DTS, you would have at least: compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; Where cygnus-ptp is more specific than iproc-ptp Then, if there is a workaround discovered in a particular SoC the driver can be updated in the future without changing the dtb. Then, the block gets added to a 4th SoC. You want us to another new compatibility string for the new SoC? If the new SoC has a bug then the driver has to be updated whether it is in uses the fallback compatible or machine_is_compatible string. There is no difference in amount of code added to a driver when a new SoC is introduced into the system that has bugs that need to be handled by the driver. The difference is in your recommendation we need to go through all the drivers used by the new SoC and add fallback compatibility strings. Not really, the fallback is what the driver should be matching by default (hence the name fallback) and if and only if you need to have a SoC-specific behavior in your driver (because of bugs, or slight differences) would you be matching this SoC-specific compatible string to capture that and key the driver behavior based off that. Then, we have to modify all the devicetree documentation for all the drivers. Then, we have to ensure that all dts files populate this new fallback string (even if it is unused). We don't see the benefit in doing any of that. Using machine_is_compatible and having less compatibility strings to deal appears much cleaner and more foolproof for all situations. When you introduce a new SoC, you would update all the bindings for the devices (not drivers) that are susceptible to be used by this SoC. If all goes well, your SoC has little bugs, and your drivers don't even need to see an update because they are already matching the fallback compatible string. That's what I understand from the suggestion but I may be totally off rails here. In this particular case, the IP is integrated within and outside a family of SoCs (the exact same block is in Capri for example). The only fallback compatablity string that makes sense is "brcm, dte-ptp-v1". When we integrate a new version of this IP we would add "brcm, dte-ptp-v2". Unless you
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Rob, On 17-06-29 03:40 PM, Rob Herring wrote: On Tue, Jun 27, 2017 at 12:10 AM, Scott Branden wrote: Hi Rob/Florian, Thanks for input but still don't see any need for SoC specific compatible stings. IP revision specific yes. On 17-06-22 06:04 PM, Florian Fainelli wrote: On 06/22/2017 05:42 PM, Scott Branden wrote: On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden wrote: Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. I don't understand what you mean by fallback compatible then. Let's say I have 3 SoCs that each contain the same ip block. You want us to add a fallback compatibility per SoC, is that correct? I think Rob meant a fallback compatibility for the particular block. E.g: brcm,iproc-ptp is the fallback compatible string, but in your SoC-specific DTS, you would have at least: compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; Where cygnus-ptp is more specific than iproc-ptp Then, if there is a workaround discovered in a particular SoC the driver can be updated in the future without changing the dtb. Then, the block gets added to a 4th SoC. You want us to another new compatibility string for the new SoC? If the new SoC has a bug then the driver has to be updated whether it is in uses the fallback compatible or machine_is_compatible string. There is no difference in amount of code added to a driver when a new SoC is introduced into the system that has bugs that need to be handled by the driver. The difference is in your recommendation we need to go through all the drivers used by the new SoC and add fallback compatibility strings. Not really, the fallback is what the driver should be matching by default (hence the name fallback) and if and only if you need to have a SoC-specific behavior in your driver (because of bugs, or slight differences) would you be matching this SoC-specific compatible string to capture that and key the driver behavior based off that. Then, we have to modify all the devicetree documentation for all the drivers. Then, we have to ensure that all dts files populate this new fallback string (even if it is unused). We don't see the benefit in doing any of that. Using machine_is_compatible and having less compatibility strings to deal appears much cleaner and more foolproof for all situations. When you introduce a new SoC, you would update all the bindings for the devices (not drivers) that are susceptible to be used by this SoC. If all goes well, your SoC has little bugs, and your drivers don't even need to see an update because they are already matching the fallback compatible string. That's what I understand from the suggestion but I may be totally off rails here. In this particular case, the IP is integrated within and outside a family of SoCs (the exact same block is in Capri for example). The only fallback compatablity string that makes sense is "brcm, dte-ptp-v1". When we integrate a new version of this IP we would add "brcm, dte-ptp-v2". Unless you have strong version control of IP versions, version numbers aren't generally reliable.
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Tue, Jun 27, 2017 at 12:10 AM, Scott Brandenwrote: > Hi Rob/Florian, > > Thanks for input but still don't see any need for SoC specific > compatible stings. IP revision specific yes. > > > On 17-06-22 06:04 PM, Florian Fainelli wrote: >> >> On 06/22/2017 05:42 PM, Scott Branden wrote: >>> >>> >>> On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden wrote: > > Hi Rob, > > > On 17-06-18 07:04 AM, Rob Herring wrote: >> >> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >>> >>> Add device tree binding documentation for the Broadcom DTE >>> PTP clock driver. >>> >>> Signed-off-by: Arun Parameswaran >>> --- >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 >>> + >>> 1 file changed, 13 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> >>> diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> new file mode 100644 >>> index 000..07590bc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> @@ -0,0 +1,13 @@ >>> +* Broadcom Digital Timing Engine(DTE) based PTP clock driver >> >> Bindings describe h/w, not drivers. >> >>> + >>> +Required properties: >>> +- compatible: should be "brcm,ptp-dte" >> >> Looks too generic. You need SoC specific compatible strings. > > Rob, could you please help me understand the use of adding SoC specific > compatible strings. > I still don't get it. > > It's my understanding that the SoC compatibility string is to future > proof > against bugs/incompatibilities > between different versions of the hardware block due to integration > issues > or any other reason. > You can then compare in your driver because the strings were already > used in > the dtb. > > That would make sense if you can't already differentiate what SoC you > are > running on. > But the SoC is already specified in the root of the device tree in the > compatible string? > Why can't you just use of_machine_is_compatible inside your driver when > needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. >>> >>> I don't understand what you mean by fallback compatible then. >>> >>> Let's say I have 3 SoCs that each contain the same ip block. >>> You want us to add a fallback compatibility per SoC, is that correct? >> >> I think Rob meant a fallback compatibility for the particular block. >> E.g: brcm,iproc-ptp is the fallback compatible string, but in your >> SoC-specific DTS, you would have at least: >> >> compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; >> >> Where cygnus-ptp is more specific than iproc-ptp >>> >>> Then, if there is a workaround discovered in a particular SoC the driver >>> can be updated in the future without changing the dtb. >>> >>> Then, the block gets added to a 4th SoC. >>> You want us to another new compatibility string for the new SoC? >>> If the new SoC has a bug then the driver has to be updated whether it is >>> in uses the fallback compatible or machine_is_compatible string. >>> >>> There is no difference in amount of code added to a driver when a new >>> SoC is introduced into the system that has bugs that need to be handled >>> by the driver. >>> >>> The difference is in your recommendation we need to go through all the >>> drivers used by the new SoC and add fallback compatibility strings. >> >> Not really, the fallback is what the driver should be matching by >> default (hence the name fallback) and if and only if you need to have a >> SoC-specific behavior in your driver (because of bugs, or slight >> differences) would you be matching this SoC-specific compatible string >> to capture that and key the driver behavior based off that. >> >>> Then, we have to modify all the devicetree documentation for all the >>> drivers. Then, we have to ensure that all dts files populate this new >>> fallback string (even if it is unused). We don't see the benefit in >>> doing any of that. Using machine_is_compatible and having less >>> compatibility strings to deal appears much cleaner and more foolproof >>> for all situations. >> >> When you introduce a new SoC, you would update all the bindings for the >> devices (not drivers) that are susceptible to be used by this
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Tue, Jun 27, 2017 at 12:10 AM, Scott Branden wrote: > Hi Rob/Florian, > > Thanks for input but still don't see any need for SoC specific > compatible stings. IP revision specific yes. > > > On 17-06-22 06:04 PM, Florian Fainelli wrote: >> >> On 06/22/2017 05:42 PM, Scott Branden wrote: >>> >>> >>> On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden wrote: > > Hi Rob, > > > On 17-06-18 07:04 AM, Rob Herring wrote: >> >> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >>> >>> Add device tree binding documentation for the Broadcom DTE >>> PTP clock driver. >>> >>> Signed-off-by: Arun Parameswaran >>> --- >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 >>> + >>> 1 file changed, 13 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> >>> diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> new file mode 100644 >>> index 000..07590bc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> @@ -0,0 +1,13 @@ >>> +* Broadcom Digital Timing Engine(DTE) based PTP clock driver >> >> Bindings describe h/w, not drivers. >> >>> + >>> +Required properties: >>> +- compatible: should be "brcm,ptp-dte" >> >> Looks too generic. You need SoC specific compatible strings. > > Rob, could you please help me understand the use of adding SoC specific > compatible strings. > I still don't get it. > > It's my understanding that the SoC compatibility string is to future > proof > against bugs/incompatibilities > between different versions of the hardware block due to integration > issues > or any other reason. > You can then compare in your driver because the strings were already > used in > the dtb. > > That would make sense if you can't already differentiate what SoC you > are > running on. > But the SoC is already specified in the root of the device tree in the > compatible string? > Why can't you just use of_machine_is_compatible inside your driver when > needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. >>> >>> I don't understand what you mean by fallback compatible then. >>> >>> Let's say I have 3 SoCs that each contain the same ip block. >>> You want us to add a fallback compatibility per SoC, is that correct? >> >> I think Rob meant a fallback compatibility for the particular block. >> E.g: brcm,iproc-ptp is the fallback compatible string, but in your >> SoC-specific DTS, you would have at least: >> >> compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; >> >> Where cygnus-ptp is more specific than iproc-ptp >>> >>> Then, if there is a workaround discovered in a particular SoC the driver >>> can be updated in the future without changing the dtb. >>> >>> Then, the block gets added to a 4th SoC. >>> You want us to another new compatibility string for the new SoC? >>> If the new SoC has a bug then the driver has to be updated whether it is >>> in uses the fallback compatible or machine_is_compatible string. >>> >>> There is no difference in amount of code added to a driver when a new >>> SoC is introduced into the system that has bugs that need to be handled >>> by the driver. >>> >>> The difference is in your recommendation we need to go through all the >>> drivers used by the new SoC and add fallback compatibility strings. >> >> Not really, the fallback is what the driver should be matching by >> default (hence the name fallback) and if and only if you need to have a >> SoC-specific behavior in your driver (because of bugs, or slight >> differences) would you be matching this SoC-specific compatible string >> to capture that and key the driver behavior based off that. >> >>> Then, we have to modify all the devicetree documentation for all the >>> drivers. Then, we have to ensure that all dts files populate this new >>> fallback string (even if it is unused). We don't see the benefit in >>> doing any of that. Using machine_is_compatible and having less >>> compatibility strings to deal appears much cleaner and more foolproof >>> for all situations. >> >> When you introduce a new SoC, you would update all the bindings for the >> devices (not drivers) that are susceptible to be used by this SoC. If >> all goes well, your SoC has little bugs, and your drivers don't even >> need to
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Rob/Florian, Thanks for input but still don't see any need for SoC specific compatible stings. IP revision specific yes. On 17-06-22 06:04 PM, Florian Fainelli wrote: On 06/22/2017 05:42 PM, Scott Branden wrote: On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Brandenwrote: Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. I don't understand what you mean by fallback compatible then. Let's say I have 3 SoCs that each contain the same ip block. You want us to add a fallback compatibility per SoC, is that correct? I think Rob meant a fallback compatibility for the particular block. E.g: brcm,iproc-ptp is the fallback compatible string, but in your SoC-specific DTS, you would have at least: compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; Where cygnus-ptp is more specific than iproc-ptp Then, if there is a workaround discovered in a particular SoC the driver can be updated in the future without changing the dtb. Then, the block gets added to a 4th SoC. You want us to another new compatibility string for the new SoC? If the new SoC has a bug then the driver has to be updated whether it is in uses the fallback compatible or machine_is_compatible string. There is no difference in amount of code added to a driver when a new SoC is introduced into the system that has bugs that need to be handled by the driver. The difference is in your recommendation we need to go through all the drivers used by the new SoC and add fallback compatibility strings. Not really, the fallback is what the driver should be matching by default (hence the name fallback) and if and only if you need to have a SoC-specific behavior in your driver (because of bugs, or slight differences) would you be matching this SoC-specific compatible string to capture that and key the driver behavior based off that. Then, we have to modify all the devicetree documentation for all the drivers. Then, we have to ensure that all dts files populate this new fallback string (even if it is unused). We don't see the benefit in doing any of that. Using machine_is_compatible and having less compatibility strings to deal appears much cleaner and more foolproof for all situations. When you introduce a new SoC, you would update all the bindings for the devices (not drivers) that are susceptible to be used by this SoC. If all goes well, your SoC has little bugs, and your drivers don't even need to see an update because they are already matching the fallback compatible string. That's what I understand from the suggestion but I may be totally off rails here. In this particular case, the IP is integrated within and outside a family of SoCs (the exact same block is in Capri for example). The only fallback compatablity string that makes sense is "brcm, dte-ptp-v1". When we integrate a new version of this IP we would add "brcm, dte-ptp-v2". Rob, I still see no need to define any other compatablity strings. Everything else can be handled with machine_is_compatible with the exact same
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Rob/Florian, Thanks for input but still don't see any need for SoC specific compatible stings. IP revision specific yes. On 17-06-22 06:04 PM, Florian Fainelli wrote: On 06/22/2017 05:42 PM, Scott Branden wrote: On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden wrote: Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. I don't understand what you mean by fallback compatible then. Let's say I have 3 SoCs that each contain the same ip block. You want us to add a fallback compatibility per SoC, is that correct? I think Rob meant a fallback compatibility for the particular block. E.g: brcm,iproc-ptp is the fallback compatible string, but in your SoC-specific DTS, you would have at least: compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; Where cygnus-ptp is more specific than iproc-ptp Then, if there is a workaround discovered in a particular SoC the driver can be updated in the future without changing the dtb. Then, the block gets added to a 4th SoC. You want us to another new compatibility string for the new SoC? If the new SoC has a bug then the driver has to be updated whether it is in uses the fallback compatible or machine_is_compatible string. There is no difference in amount of code added to a driver when a new SoC is introduced into the system that has bugs that need to be handled by the driver. The difference is in your recommendation we need to go through all the drivers used by the new SoC and add fallback compatibility strings. Not really, the fallback is what the driver should be matching by default (hence the name fallback) and if and only if you need to have a SoC-specific behavior in your driver (because of bugs, or slight differences) would you be matching this SoC-specific compatible string to capture that and key the driver behavior based off that. Then, we have to modify all the devicetree documentation for all the drivers. Then, we have to ensure that all dts files populate this new fallback string (even if it is unused). We don't see the benefit in doing any of that. Using machine_is_compatible and having less compatibility strings to deal appears much cleaner and more foolproof for all situations. When you introduce a new SoC, you would update all the bindings for the devices (not drivers) that are susceptible to be used by this SoC. If all goes well, your SoC has little bugs, and your drivers don't even need to see an update because they are already matching the fallback compatible string. That's what I understand from the suggestion but I may be totally off rails here. In this particular case, the IP is integrated within and outside a family of SoCs (the exact same block is in Capri for example). The only fallback compatablity string that makes sense is "brcm, dte-ptp-v1". When we integrate a new version of this IP we would add "brcm, dte-ptp-v2". Rob, I still see no need to define any other compatablity strings. Everything else can be handled with machine_is_compatible with the exact same amount of code change to the driver? Adding the additional
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 06/22/2017 05:42 PM, Scott Branden wrote: > > > On 17-06-21 08:19 PM, Rob Herring wrote: >> On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden >>wrote: >>> Hi Rob, >>> >>> >>> On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: > Add device tree binding documentation for the Broadcom DTE > PTP clock driver. > > Signed-off-by: Arun Parameswaran > --- >Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 > + >1 file changed, 13 insertions(+) >create mode 100644 > Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > > diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > new file mode 100644 > index 000..07590bc > --- /dev/null > +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > @@ -0,0 +1,13 @@ > +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. > + > +Required properties: > +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. >>> >>> Rob, could you please help me understand the use of adding SoC specific >>> compatible strings. >>> I still don't get it. >>> >>> It's my understanding that the SoC compatibility string is to future >>> proof >>> against bugs/incompatibilities >>> between different versions of the hardware block due to integration >>> issues >>> or any other reason. >>> You can then compare in your driver because the strings were already >>> used in >>> the dtb. >>> >>> That would make sense if you can't already differentiate what SoC you >>> are >>> running on. >>> But the SoC is already specified in the root of the device tree in the >>> compatible string? >>> Why can't you just use of_machine_is_compatible inside your driver when >>> needed? >> Use of of_machine_is_compatible in drivers will result in the same >> mess we had with machine_is_X defines pre-DT. It practically >> guarantees that you must update the driver for a new SoC (with >> fallback compatibles you don't). Plus the matching logic for >> of_machine_is_compatible is open coded logic in every driver which is >> worse IMO than having a standard match table. > I don't understand what you mean by fallback compatible then. > > Let's say I have 3 SoCs that each contain the same ip block. > You want us to add a fallback compatibility per SoC, is that correct? I think Rob meant a fallback compatibility for the particular block. E.g: brcm,iproc-ptp is the fallback compatible string, but in your SoC-specific DTS, you would have at least: compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; Where cygnus-ptp is more specific than iproc-ptp > > Then, if there is a workaround discovered in a particular SoC the driver > can be updated in the future without changing the dtb. > > Then, the block gets added to a 4th SoC. > You want us to another new compatibility string for the new SoC? > If the new SoC has a bug then the driver has to be updated whether it is > in uses the fallback compatible or machine_is_compatible string. > > There is no difference in amount of code added to a driver when a new > SoC is introduced into the system that has bugs that need to be handled > by the driver. > > The difference is in your recommendation we need to go through all the > drivers used by the new SoC and add fallback compatibility strings. Not really, the fallback is what the driver should be matching by default (hence the name fallback) and if and only if you need to have a SoC-specific behavior in your driver (because of bugs, or slight differences) would you be matching this SoC-specific compatible string to capture that and key the driver behavior based off that. > Then, we have to modify all the devicetree documentation for all the > drivers. Then, we have to ensure that all dts files populate this new > fallback string (even if it is unused). We don't see the benefit in > doing any of that. Using machine_is_compatible and having less > compatibility strings to deal appears much cleaner and more foolproof > for all situations. When you introduce a new SoC, you would update all the bindings for the devices (not drivers) that are susceptible to be used by this SoC. If all goes well, your SoC has little bugs, and your drivers don't even need to see an update because they are already matching the fallback compatible string. That's what I understand from the suggestion but I may be totally off rails here. -- Florian
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 06/22/2017 05:42 PM, Scott Branden wrote: > > > On 17-06-21 08:19 PM, Rob Herring wrote: >> On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden >> wrote: >>> Hi Rob, >>> >>> >>> On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: > Add device tree binding documentation for the Broadcom DTE > PTP clock driver. > > Signed-off-by: Arun Parameswaran > --- >Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 > + >1 file changed, 13 insertions(+) >create mode 100644 > Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > > diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > new file mode 100644 > index 000..07590bc > --- /dev/null > +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > @@ -0,0 +1,13 @@ > +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. > + > +Required properties: > +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. >>> >>> Rob, could you please help me understand the use of adding SoC specific >>> compatible strings. >>> I still don't get it. >>> >>> It's my understanding that the SoC compatibility string is to future >>> proof >>> against bugs/incompatibilities >>> between different versions of the hardware block due to integration >>> issues >>> or any other reason. >>> You can then compare in your driver because the strings were already >>> used in >>> the dtb. >>> >>> That would make sense if you can't already differentiate what SoC you >>> are >>> running on. >>> But the SoC is already specified in the root of the device tree in the >>> compatible string? >>> Why can't you just use of_machine_is_compatible inside your driver when >>> needed? >> Use of of_machine_is_compatible in drivers will result in the same >> mess we had with machine_is_X defines pre-DT. It practically >> guarantees that you must update the driver for a new SoC (with >> fallback compatibles you don't). Plus the matching logic for >> of_machine_is_compatible is open coded logic in every driver which is >> worse IMO than having a standard match table. > I don't understand what you mean by fallback compatible then. > > Let's say I have 3 SoCs that each contain the same ip block. > You want us to add a fallback compatibility per SoC, is that correct? I think Rob meant a fallback compatibility for the particular block. E.g: brcm,iproc-ptp is the fallback compatible string, but in your SoC-specific DTS, you would have at least: compatible = "brcm,cygnus-ptp", "brcm,iproc-ptp"; Where cygnus-ptp is more specific than iproc-ptp > > Then, if there is a workaround discovered in a particular SoC the driver > can be updated in the future without changing the dtb. > > Then, the block gets added to a 4th SoC. > You want us to another new compatibility string for the new SoC? > If the new SoC has a bug then the driver has to be updated whether it is > in uses the fallback compatible or machine_is_compatible string. > > There is no difference in amount of code added to a driver when a new > SoC is introduced into the system that has bugs that need to be handled > by the driver. > > The difference is in your recommendation we need to go through all the > drivers used by the new SoC and add fallback compatibility strings. Not really, the fallback is what the driver should be matching by default (hence the name fallback) and if and only if you need to have a SoC-specific behavior in your driver (because of bugs, or slight differences) would you be matching this SoC-specific compatible string to capture that and key the driver behavior based off that. > Then, we have to modify all the devicetree documentation for all the > drivers. Then, we have to ensure that all dts files populate this new > fallback string (even if it is unused). We don't see the benefit in > doing any of that. Using machine_is_compatible and having less > compatibility strings to deal appears much cleaner and more foolproof > for all situations. When you introduce a new SoC, you would update all the bindings for the devices (not drivers) that are susceptible to be used by this SoC. If all goes well, your SoC has little bugs, and your drivers don't even need to see an update because they are already matching the fallback compatible string. That's what I understand from the suggestion but I may be totally off rails here. -- Florian
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Brandenwrote: Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. I don't understand what you mean by fallback compatible then. Let's say I have 3 SoCs that each contain the same ip block. You want us to add a fallback compatibility per SoC, is that correct? Then, if there is a workaround discovered in a particular SoC the driver can be updated in the future without changing the dtb. Then, the block gets added to a 4th SoC. You want us to another new compatibility string for the new SoC? If the new SoC has a bug then the driver has to be updated whether it is in uses the fallback compatible or machine_is_compatible string. There is no difference in amount of code added to a driver when a new SoC is introduced into the system that has bugs that need to be handled by the driver. The difference is in your recommendation we need to go through all the drivers used by the new SoC and add fallback compatibility strings. Then, we have to modify all the devicetree documentation for all the drivers. Then, we have to ensure that all dts files populate this new fallback string (even if it is unused). We don't see the benefit in doing any of that. Using machine_is_compatible and having less compatibility strings to deal appears much cleaner and more foolproof for all situations. Please explain what I'm missing. I see other drivers already following the of_machine_is_compatible approach and it makes more sense to me than adding SoC specific compatible strings into every driver. If the IP blocks are all the same, you don't have to add them to any drivers, just add fallback compatibles. Rob Regards, Scott
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 17-06-21 08:19 PM, Rob Herring wrote: On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden wrote: Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. I don't understand what you mean by fallback compatible then. Let's say I have 3 SoCs that each contain the same ip block. You want us to add a fallback compatibility per SoC, is that correct? Then, if there is a workaround discovered in a particular SoC the driver can be updated in the future without changing the dtb. Then, the block gets added to a 4th SoC. You want us to another new compatibility string for the new SoC? If the new SoC has a bug then the driver has to be updated whether it is in uses the fallback compatible or machine_is_compatible string. There is no difference in amount of code added to a driver when a new SoC is introduced into the system that has bugs that need to be handled by the driver. The difference is in your recommendation we need to go through all the drivers used by the new SoC and add fallback compatibility strings. Then, we have to modify all the devicetree documentation for all the drivers. Then, we have to ensure that all dts files populate this new fallback string (even if it is unused). We don't see the benefit in doing any of that. Using machine_is_compatible and having less compatibility strings to deal appears much cleaner and more foolproof for all situations. Please explain what I'm missing. I see other drivers already following the of_machine_is_compatible approach and it makes more sense to me than adding SoC specific compatible strings into every driver. If the IP blocks are all the same, you don't have to add them to any drivers, just add fallback compatibles. Rob Regards, Scott
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Tue, Jun 20, 2017 at 3:48 PM, Scott Brandenwrote: > Hi Rob, > > > On 17-06-18 07:04 AM, Rob Herring wrote: >> >> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >>> >>> Add device tree binding documentation for the Broadcom DTE >>> PTP clock driver. >>> >>> Signed-off-by: Arun Parameswaran >>> --- >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 >>> + >>> 1 file changed, 13 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> >>> diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> new file mode 100644 >>> index 000..07590bc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> @@ -0,0 +1,13 @@ >>> +* Broadcom Digital Timing Engine(DTE) based PTP clock driver >> >> Bindings describe h/w, not drivers. >> >>> + >>> +Required properties: >>> +- compatible: should be "brcm,ptp-dte" >> >> Looks too generic. You need SoC specific compatible strings. > > > Rob, could you please help me understand the use of adding SoC specific > compatible strings. > I still don't get it. > > It's my understanding that the SoC compatibility string is to future proof > against bugs/incompatibilities > between different versions of the hardware block due to integration issues > or any other reason. > You can then compare in your driver because the strings were already used in > the dtb. > > That would make sense if you can't already differentiate what SoC you are > running on. > But the SoC is already specified in the root of the device tree in the > compatible string? > Why can't you just use of_machine_is_compatible inside your driver when > needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. > > Please explain what I'm missing. I see other drivers already following the > of_machine_is_compatible > approach and it makes more sense to me than adding SoC specific compatible > strings into every > driver. If the IP blocks are all the same, you don't have to add them to any drivers, just add fallback compatibles. Rob
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Tue, Jun 20, 2017 at 3:48 PM, Scott Branden wrote: > Hi Rob, > > > On 17-06-18 07:04 AM, Rob Herring wrote: >> >> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >>> >>> Add device tree binding documentation for the Broadcom DTE >>> PTP clock driver. >>> >>> Signed-off-by: Arun Parameswaran >>> --- >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 >>> + >>> 1 file changed, 13 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> >>> diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> new file mode 100644 >>> index 000..07590bc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >>> @@ -0,0 +1,13 @@ >>> +* Broadcom Digital Timing Engine(DTE) based PTP clock driver >> >> Bindings describe h/w, not drivers. >> >>> + >>> +Required properties: >>> +- compatible: should be "brcm,ptp-dte" >> >> Looks too generic. You need SoC specific compatible strings. > > > Rob, could you please help me understand the use of adding SoC specific > compatible strings. > I still don't get it. > > It's my understanding that the SoC compatibility string is to future proof > against bugs/incompatibilities > between different versions of the hardware block due to integration issues > or any other reason. > You can then compare in your driver because the strings were already used in > the dtb. > > That would make sense if you can't already differentiate what SoC you are > running on. > But the SoC is already specified in the root of the device tree in the > compatible string? > Why can't you just use of_machine_is_compatible inside your driver when > needed? Use of of_machine_is_compatible in drivers will result in the same mess we had with machine_is_X defines pre-DT. It practically guarantees that you must update the driver for a new SoC (with fallback compatibles you don't). Plus the matching logic for of_machine_is_compatible is open coded logic in every driver which is worse IMO than having a standard match table. > > Please explain what I'm missing. I see other drivers already following the > of_machine_is_compatible > approach and it makes more sense to me than adding SoC specific compatible > strings into every > driver. If the IP blocks are all the same, you don't have to add them to any drivers, just add fallback compatibles. Rob
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran--- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Please explain what I'm missing. I see other drivers already following the of_machine_is_compatible approach and it makes more sense to me than adding SoC specific compatible strings into every driver. Regards, Scott
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Rob, On 17-06-18 07:04 AM, Rob Herring wrote: On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. + +Required properties: +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. Rob, could you please help me understand the use of adding SoC specific compatible strings. I still don't get it. It's my understanding that the SoC compatibility string is to future proof against bugs/incompatibilities between different versions of the hardware block due to integration issues or any other reason. You can then compare in your driver because the strings were already used in the dtb. That would make sense if you can't already differentiate what SoC you are running on. But the SoC is already specified in the root of the device tree in the compatible string? Why can't you just use of_machine_is_compatible inside your driver when needed? Please explain what I'm missing. I see other drivers already following the of_machine_is_compatible approach and it makes more sense to me than adding SoC specific compatible strings into every driver. Regards, Scott
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi David, Rob, I will address all of Rob's comments below. Since a part of the patch was applied to 'net-next', would you like me to send a new patch (based on the applied one), or a 'V2' of this patch ? Thanks Arun On 17-06-18 07:04 AM, Rob Herring wrote: > On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >> Add device tree binding documentation for the Broadcom DTE >> PTP clock driver. >> >> Signed-off-by: Arun Parameswaran>> --- >> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + >> 1 file changed, 13 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> >> diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> new file mode 100644 >> index 000..07590bc >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> @@ -0,0 +1,13 @@ >> +* Broadcom Digital Timing Engine(DTE) based PTP clock driver > > Bindings describe h/w, not drivers. > >> + >> +Required properties: >> +- compatible: should be "brcm,ptp-dte" > > Looks too generic. You need SoC specific compatible strings. > >> +- reg: address and length of the DTE block's NCO registers >> + >> +Example: >> + >> +ptp_dte: ptp_dte@180af650 { > > Don't use '_' in node names. > >> +compatible = "brcm,ptp-dte"; >> +reg = <0x180af650 0x10>; >> +status = "okay"; >> +}; >> -- >> 1.9.1 >>
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi David, Rob, I will address all of Rob's comments below. Since a part of the patch was applied to 'net-next', would you like me to send a new patch (based on the applied one), or a 'V2' of this patch ? Thanks Arun On 17-06-18 07:04 AM, Rob Herring wrote: > On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >> Add device tree binding documentation for the Broadcom DTE >> PTP clock driver. >> >> Signed-off-by: Arun Parameswaran >> --- >> Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + >> 1 file changed, 13 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> >> diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> new file mode 100644 >> index 000..07590bc >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt >> @@ -0,0 +1,13 @@ >> +* Broadcom Digital Timing Engine(DTE) based PTP clock driver > > Bindings describe h/w, not drivers. > >> + >> +Required properties: >> +- compatible: should be "brcm,ptp-dte" > > Looks too generic. You need SoC specific compatible strings. > >> +- reg: address and length of the DTE block's NCO registers >> + >> +Example: >> + >> +ptp_dte: ptp_dte@180af650 { > > Don't use '_' in node names. > >> +compatible = "brcm,ptp-dte"; >> +reg = <0x180af650 0x10>; >> +status = "okay"; >> +}; >> -- >> 1.9.1 >>
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: > Add device tree binding documentation for the Broadcom DTE > PTP clock driver. > > Signed-off-by: Arun Parameswaran> --- > Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + > 1 file changed, 13 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > > diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > new file mode 100644 > index 000..07590bc > --- /dev/null > +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > @@ -0,0 +1,13 @@ > +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. > + > +Required properties: > +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. > +- reg: address and length of the DTE block's NCO registers > + > +Example: > + > +ptp_dte: ptp_dte@180af650 { Don't use '_' in node names. > + compatible = "brcm,ptp-dte"; > + reg = <0x180af650 0x10>; > + status = "okay"; > +}; > -- > 1.9.1 >
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: > Add device tree binding documentation for the Broadcom DTE > PTP clock driver. > > Signed-off-by: Arun Parameswaran > --- > Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + > 1 file changed, 13 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > > diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > new file mode 100644 > index 000..07590bc > --- /dev/null > +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt > @@ -0,0 +1,13 @@ > +* Broadcom Digital Timing Engine(DTE) based PTP clock driver Bindings describe h/w, not drivers. > + > +Required properties: > +- compatible: should be "brcm,ptp-dte" Looks too generic. You need SoC specific compatible strings. > +- reg: address and length of the DTE block's NCO registers > + > +Example: > + > +ptp_dte: ptp_dte@180af650 { Don't use '_' in node names. > + compatible = "brcm,ptp-dte"; > + reg = <0x180af650 0x10>; > + status = "okay"; > +}; > -- > 1.9.1 >
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 17-06-14 11:18 AM, Ray Jui wrote: > > > On 6/13/17 10:46 AM, Arun Parameswaran wrote: >> >> Hi Richard, >> >> On 17-06-12 10:09 PM, Richard Cochran wrote: >>> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: +Example: + +ptp_dte: ptp_dte@180af650 { + compatible = "brcm,ptp-dte"; + reg = <0x180af650 0x10>; + status = "okay"; +}; >>> This patch set looks okay, as far as it goes, but how does one >>> actually use the clock? >>> >>> How does one connect this node to an Ethernet MAC, for example? >> In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's >> 1588 Shim NCO (used for time-stamping the packets). We are working on getting >> all the future SoC's synced this way (for Ethernet). >> > > So apparently you do not need any phandle based DT binding defined for > the connection between the Ethernet device node and the PTP device node? > Correct. This driver doesn't need any phandle. The Ethernet driver, in SoC's where the hardware link is not available, might need the phandle to this driver. >> In other scenarios, we use an user-space app to perform the sync based on >> time-stamps from external hardware sources like the Wi-Fi. >>> Thanks, >>> Richard >>
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 17-06-14 11:18 AM, Ray Jui wrote: > > > On 6/13/17 10:46 AM, Arun Parameswaran wrote: >> >> Hi Richard, >> >> On 17-06-12 10:09 PM, Richard Cochran wrote: >>> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: +Example: + +ptp_dte: ptp_dte@180af650 { + compatible = "brcm,ptp-dte"; + reg = <0x180af650 0x10>; + status = "okay"; +}; >>> This patch set looks okay, as far as it goes, but how does one >>> actually use the clock? >>> >>> How does one connect this node to an Ethernet MAC, for example? >> In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's >> 1588 Shim NCO (used for time-stamping the packets). We are working on getting >> all the future SoC's synced this way (for Ethernet). >> > > So apparently you do not need any phandle based DT binding defined for > the connection between the Ethernet device node and the PTP device node? > Correct. This driver doesn't need any phandle. The Ethernet driver, in SoC's where the hardware link is not available, might need the phandle to this driver. >> In other scenarios, we use an user-space app to perform the sync based on >> time-stamps from external hardware sources like the Wi-Fi. >>> Thanks, >>> Richard >>
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 17-06-14 11:18 AM, Ray Jui wrote: > > > On 6/13/17 10:46 AM, Arun Parameswaran wrote: >> >> Hi Richard, >> >> On 17-06-12 10:09 PM, Richard Cochran wrote: >>> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: +Example: + +ptp_dte: ptp_dte@180af650 { + compatible = "brcm,ptp-dte"; + reg = <0x180af650 0x10>; + status = "okay"; +}; >>> This patch set looks okay, as far as it goes, but how does one >>> actually use the clock? >>> >>> How does one connect this node to an Ethernet MAC, for example? >> In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's >> 1588 Shim NCO (used for time-stamping the packets). We are working on getting >> all the future SoC's synced this way (for Ethernet). >> > > So apparently you do not need any phandle based DT binding defined for > the connection between the Ethernet device node and the PTP device node? > Correct. This driver doesn't need any phandle. The Ethernet/Switch driver, in the SoC's where the hardware link is not available, might need the phandle to this driver. >> In other scenarios, we use an user-space app to perform the sync based on >> time-stamps from external hardware sources like the Wi-Fi. >>> Thanks, >>> Richard >>
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 17-06-14 11:18 AM, Ray Jui wrote: > > > On 6/13/17 10:46 AM, Arun Parameswaran wrote: >> >> Hi Richard, >> >> On 17-06-12 10:09 PM, Richard Cochran wrote: >>> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: +Example: + +ptp_dte: ptp_dte@180af650 { + compatible = "brcm,ptp-dte"; + reg = <0x180af650 0x10>; + status = "okay"; +}; >>> This patch set looks okay, as far as it goes, but how does one >>> actually use the clock? >>> >>> How does one connect this node to an Ethernet MAC, for example? >> In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's >> 1588 Shim NCO (used for time-stamping the packets). We are working on getting >> all the future SoC's synced this way (for Ethernet). >> > > So apparently you do not need any phandle based DT binding defined for > the connection between the Ethernet device node and the PTP device node? > Correct. This driver doesn't need any phandle. The Ethernet/Switch driver, in the SoC's where the hardware link is not available, might need the phandle to this driver. >> In other scenarios, we use an user-space app to perform the sync based on >> time-stamps from external hardware sources like the Wi-Fi. >>> Thanks, >>> Richard >>
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 6/13/17 10:46 AM, Arun Parameswaran wrote: > > Hi Richard, > > On 17-06-12 10:09 PM, Richard Cochran wrote: >> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >>> +Example: >>> + >>> +ptp_dte: ptp_dte@180af650 { >>> + compatible = "brcm,ptp-dte"; >>> + reg = <0x180af650 0x10>; >>> + status = "okay"; >>> +}; >> This patch set looks okay, as far as it goes, but how does one >> actually use the clock? >> >> How does one connect this node to an Ethernet MAC, for example? > In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's > 1588 Shim NCO (used for time-stamping the packets). We are working on getting > all the future SoC's synced this way (for Ethernet). > So apparently you do not need any phandle based DT binding defined for the connection between the Ethernet device node and the PTP device node? > In other scenarios, we use an user-space app to perform the sync based on > time-stamps from external hardware sources like the Wi-Fi. >> Thanks, >> Richard >
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On 6/13/17 10:46 AM, Arun Parameswaran wrote: > > Hi Richard, > > On 17-06-12 10:09 PM, Richard Cochran wrote: >> On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >>> +Example: >>> + >>> +ptp_dte: ptp_dte@180af650 { >>> + compatible = "brcm,ptp-dte"; >>> + reg = <0x180af650 0x10>; >>> + status = "okay"; >>> +}; >> This patch set looks okay, as far as it goes, but how does one >> actually use the clock? >> >> How does one connect this node to an Ethernet MAC, for example? > In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's > 1588 Shim NCO (used for time-stamping the packets). We are working on getting > all the future SoC's synced this way (for Ethernet). > So apparently you do not need any phandle based DT binding defined for the connection between the Ethernet device node and the PTP device node? > In other scenarios, we use an user-space app to perform the sync based on > time-stamps from external hardware sources like the Wi-Fi. >> Thanks, >> Richard >
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Richard, On 17-06-12 10:09 PM, Richard Cochran wrote: > On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >> +Example: >> + >> +ptp_dte: ptp_dte@180af650 { >> +compatible = "brcm,ptp-dte"; >> +reg = <0x180af650 0x10>; >> +status = "okay"; >> +}; > This patch set looks okay, as far as it goes, but how does one > actually use the clock? > > How does one connect this node to an Ethernet MAC, for example? In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's 1588 Shim NCO (used for time-stamping the packets). We are working on getting all the future SoC's synced this way (for Ethernet). In other scenarios, we use an user-space app to perform the sync based on time-stamps from external hardware sources like the Wi-Fi. > Thanks, > Richard
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Hi Richard, On 17-06-12 10:09 PM, Richard Cochran wrote: > On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: >> +Example: >> + >> +ptp_dte: ptp_dte@180af650 { >> +compatible = "brcm,ptp-dte"; >> +reg = <0x180af650 0x10>; >> +status = "okay"; >> +}; > This patch set looks okay, as far as it goes, but how does one > actually use the clock? > > How does one connect this node to an Ethernet MAC, for example? In some of the SoC's, the DTE's NCO is synced in hardware with the Ethernet's 1588 Shim NCO (used for time-stamping the packets). We are working on getting all the future SoC's synced this way (for Ethernet). In other scenarios, we use an user-space app to perform the sync based on time-stamps from external hardware sources like the Wi-Fi. > Thanks, > Richard
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: > +Example: > + > +ptp_dte: ptp_dte@180af650 { > + compatible = "brcm,ptp-dte"; > + reg = <0x180af650 0x10>; > + status = "okay"; > +}; This patch set looks okay, as far as it goes, but how does one actually use the clock? How does one connect this node to an Ethernet MAC, for example? Thanks, Richard
Re: [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
On Mon, Jun 12, 2017 at 01:26:00PM -0700, Arun Parameswaran wrote: > +Example: > + > +ptp_dte: ptp_dte@180af650 { > + compatible = "brcm,ptp-dte"; > + reg = <0x180af650 0x10>; > + status = "okay"; > +}; This patch set looks okay, as far as it goes, but how does one actually use the clock? How does one connect this node to an Ethernet MAC, for example? Thanks, Richard
[PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran--- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver + +Required properties: +- compatible: should be "brcm,ptp-dte" +- reg: address and length of the DTE block's NCO registers + +Example: + +ptp_dte: ptp_dte@180af650 { + compatible = "brcm,ptp-dte"; + reg = <0x180af650 0x10>; + status = "okay"; +}; -- 1.9.1
[PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
Add device tree binding documentation for the Broadcom DTE PTP clock driver. Signed-off-by: Arun Parameswaran --- Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt new file mode 100644 index 000..07590bc --- /dev/null +++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt @@ -0,0 +1,13 @@ +* Broadcom Digital Timing Engine(DTE) based PTP clock driver + +Required properties: +- compatible: should be "brcm,ptp-dte" +- reg: address and length of the DTE block's NCO registers + +Example: + +ptp_dte: ptp_dte@180af650 { + compatible = "brcm,ptp-dte"; + reg = <0x180af650 0x10>; + status = "okay"; +}; -- 1.9.1