Good day to all,
The CS specification defines replicators that are not showing on the AMBA bus and can't be configured - they simply provide a hard wire between components. Earlier this week someone wrote to me on the open mailing list asking for guidance in dealing with a new type of funnel their company has introduced in a design. Just like a replicator it doesn't show up on the AMBA bus and can't be configured.
It makes sense to deal with this new component the same way we do with the above-mentioned replicator, i.e use a platform driver that does nothing and add it to the CS topology when specified in the device tree. It also make sense to re-use the same code [1] for that.
But moving forward with this also means we need to re-brand the current coresight-replicator.c file, along with adding new "compatible" strings. Here is what I suggest:
1) Keep the "arm,coresight-replicator" compatible string for backward compatibility (I'm seriously tempted not to [2]). 2) Add a new "arm,coresight-hw-replicator" compatible string for replicators that don't show up on the AMBA bus. 3) Add a new "arm,coresight-hw-funnel" compatible string for funnels that don't show up on the AMBA bus. 4) Use a match table to deal with the above two new compatible string and the old one (if we keep it around). 5) Rename coresight-replicator.c to coresight-hw-link.c.
I'm reaching out to the list because once we have a naming convention it is hard to modify it. Let me know what you think of the above.
Thanks, Mathieu
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/hwtracing/coresight/c... [2]. We could also delete the old "arm,coresight-replicator" and make the changes in the DT for current users. People that haven't upstream their in-house solution will have to adapt, but that's something that happens with all the out-of-tree code.
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Good day to all,
The CS specification defines replicators that are not showing on the AMBA bus and can't be configured - they simply provide a hard wire between components. Earlier this week someone wrote to me on the open mailing list asking for guidance in dealing with a new type of funnel their company has introduced in a design. Just like a replicator it doesn't show up on the AMBA bus and can't be configured.
It makes sense to deal with this new component the same way we do with the above-mentioned replicator, i.e use a platform driver that does nothing and add it to the CS topology when specified in the device tree. It also make sense to re-use the same code [1] for that.
But moving forward with this also means we need to re-brand the current coresight-replicator.c file, along with adding new "compatible" strings. Here is what I suggest:
- Keep the "arm,coresight-replicator" compatible string for backward
compatibility (I'm seriously tempted not to [2]). 2) Add a new "arm,coresight-hw-replicator" compatible string for replicators that don't show up on the AMBA bus. 3) Add a new "arm,coresight-hw-funnel" compatible string for funnels that don't show up on the AMBA bus. 4) Use a match table to deal with the above two new compatible string and the old one (if we keep it around). 5) Rename coresight-replicator.c to coresight-hw-link.c.
I'm reaching out to the list because once we have a naming convention it is hard to modify it. Let me know what you think of the above.
Thanks, Mathieu
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/hwtracing/coresight/c... [2]. We could also delete the old "arm,coresight-replicator" and make the changes in the DT for current users. People that haven't upstream their in-house solution will have to adapt, but that's something that happens with all the out-of-tree code.
I'd rather not introduce incompatibility when it's strictly not necessary.
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Alternately, the devm_ioremap_resource() callsite in funnel_probe() could check for the specific error that represents no reg property found, assume it's a 'dummy funnel', and just continue (and not error out), but not register any link_ops.
Kim
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Good day to all,
The CS specification defines replicators that are not showing on the AMBA bus and can't be configured - they simply provide a hard wire between components. Earlier this week someone wrote to me on the open mailing list asking for guidance in dealing with a new type of funnel their company has introduced in a design. Just like a replicator it doesn't show up on the AMBA bus and can't be configured.
It makes sense to deal with this new component the same way we do with the above-mentioned replicator, i.e use a platform driver that does nothing and add it to the CS topology when specified in the device tree. It also make sense to re-use the same code [1] for that.
But moving forward with this also means we need to re-brand the current coresight-replicator.c file, along with adding new "compatible" strings. Here is what I suggest:
- Keep the "arm,coresight-replicator" compatible string for backward
compatibility (I'm seriously tempted not to [2]). 2) Add a new "arm,coresight-hw-replicator" compatible string for replicators that don't show up on the AMBA bus. 3) Add a new "arm,coresight-hw-funnel" compatible string for funnels that don't show up on the AMBA bus. 4) Use a match table to deal with the above two new compatible string and the old one (if we keep it around). 5) Rename coresight-replicator.c to coresight-hw-link.c.
I'm reaching out to the list because once we have a naming convention it is hard to modify it. Let me know what you think of the above.
Thanks, Mathieu
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/hwtracing/coresight/c... [2]. We could also delete the old "arm,coresight-replicator" and make the changes in the DT for current users. People that haven't upstream their in-house solution will have to adapt, but that's something that happens with all the out-of-tree code.
I'd rather not introduce incompatibility when it's strictly not necessary.
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
Alternately, the devm_ioremap_resource() callsite in funnel_probe() could check for the specific error that represents no reg property found, assume it's a 'dummy funnel', and just continue (and not error out), but not register any link_ops.
Kim
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The probe function can test for the presence of a reg property, and if not found, assume it's a node representing one of these non-configurable devices.
Kim
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
The probe function can test for the presence of a reg property, and if not found, assume it's a node representing one of these non-configurable devices.
Kim
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
Kim
Hi Mathieu, Kim
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Kim _______________________________________________ CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
I'd be inclined to keep coresight-replicator for the non-programmable version, and perhaps in the same sense that we have a coresight-dynamic-replicator for the programmable version, introduce a new name such as coresight-auto-funnel / coresight-concentrator for this non-programmable variant.
I'd still use the same non-prog platform driver code as you suggest - and change the driver source file name if you like, but reduce the amount of DT churn by avoiding a new name for an existing component.
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
Regards
Mike
OK - I need to pay more attention to the CS specs - non-programmable funnels are indeed an arm option. Guess I've never been particularly interested in them before!
Mike
On 13 March 2018 at 23:15, Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu, Kim
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Kim _______________________________________________ CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
I'd be inclined to keep coresight-replicator for the non-programmable version, and perhaps in the same sense that we have a coresight-dynamic-replicator for the programmable version, introduce a new name such as coresight-auto-funnel / coresight-concentrator for this non-programmable variant.
I'd still use the same non-prog platform driver code as you suggest - and change the driver source file name if you like, but reduce the amount of DT churn by avoiding a new name for an existing component.
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
Regards
Mike
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
On 13 March 2018 at 17:22, Mike Leach mike.leach@linaro.org wrote:
OK - I need to pay more attention to the CS specs - non-programmable funnels are indeed an arm option. Guess I've never been particularly interested in them before!
Perfect - that is something I've been meaning to look into. I was also wondering... Do non-programmable funnel and replicators need a clock to work or is this something that is implementation dependant? The datasheet doesn't say anything and I'm thinking that it could go both ways.
Mathieu
Mike
On 13 March 2018 at 23:15, Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu, Kim
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote: > On Mon, 12 Mar 2018 11:28:33 -0600 > Mathieu Poirier mathieu.poirier@linaro.org wrote: > > Is it possible to keep everything equal (no new compatibles or other > changes), and have functions like funnel_enable_hw, funnel_disable_hw > do something like this instead?: > > if (!drvdata->base) > return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Kim _______________________________________________ CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
I'd be inclined to keep coresight-replicator for the non-programmable version, and perhaps in the same sense that we have a coresight-dynamic-replicator for the programmable version, introduce a new name such as coresight-auto-funnel / coresight-concentrator for this non-programmable variant.
I'd still use the same non-prog platform driver code as you suggest - and change the driver source file name if you like, but reduce the amount of DT churn by avoiding a new name for an existing component.
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
Regards
Mike
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
On 14 March 2018 at 15:51, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 17:22, Mike Leach mike.leach@linaro.org wrote:
OK - I need to pay more attention to the CS specs - non-programmable funnels are indeed an arm option. Guess I've never been particularly interested in them before!
Perfect - that is something I've been meaning to look into. I was also wondering... Do non-programmable funnel and replicators need a clock to work or is this something that is implementation dependant? The datasheet doesn't say anything and I'm thinking that it could go both ways.
Mathieu
Clocks are needed - the non-programmable components are essentially the same with default values in the registers that you cannot see / program . Mike.
Mike
On 13 March 2018 at 23:15, Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu, Kim
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
> On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote: > > On Mon, 12 Mar 2018 11:28:33 -0600 > > Mathieu Poirier mathieu.poirier@linaro.org wrote: > > > > Is it possible to keep everything equal (no new compatibles or other > > changes), and have functions like funnel_enable_hw, funnel_disable_hw > > do something like this instead?: > > > > if (!drvdata->base) > > return; > > Not that I'm aware of - a funnel device won't get probed if it doesn't > have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Kim _______________________________________________ CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
I'd be inclined to keep coresight-replicator for the non-programmable version, and perhaps in the same sense that we have a coresight-dynamic-replicator for the programmable version, introduce a new name such as coresight-auto-funnel / coresight-concentrator for this non-programmable variant.
I'd still use the same non-prog platform driver code as you suggest - and change the driver source file name if you like, but reduce the amount of DT churn by avoiding a new name for an existing component.
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
Regards
Mike
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
On 14 March 2018 at 09:56, Mike Leach mike.leach@linaro.org wrote:
On 14 March 2018 at 15:51, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 17:22, Mike Leach mike.leach@linaro.org wrote:
OK - I need to pay more attention to the CS specs - non-programmable funnels are indeed an arm option. Guess I've never been particularly interested in them before!
Perfect - that is something I've been meaning to look into. I was also wondering... Do non-programmable funnel and replicators need a clock to work or is this something that is implementation dependant? The datasheet doesn't say anything and I'm thinking that it could go both ways.
Mathieu
Clocks are needed - the non-programmable components are essentially the same with default values in the registers that you cannot see / program
Swell - thanks
. Mike.
Mike
On 13 March 2018 at 23:15, Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu, Kim
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote: > On Tue, 13 Mar 2018 09:40:14 -0600 > Mathieu Poirier mathieu.poirier@linaro.org wrote: > >> On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote: >> > On Mon, 12 Mar 2018 11:28:33 -0600 >> > Mathieu Poirier mathieu.poirier@linaro.org wrote: >> > >> > Is it possible to keep everything equal (no new compatibles or other >> > changes), and have functions like funnel_enable_hw, funnel_disable_hw >> > do something like this instead?: >> > >> > if (!drvdata->base) >> > return; >> >> Not that I'm aware of - a funnel device won't get probed if it doesn't >> have a valid 'reg' property to poke at. > > All a node needs to contain to get its driver probed is the compatible > property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Kim _______________________________________________ CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
I'd be inclined to keep coresight-replicator for the non-programmable version, and perhaps in the same sense that we have a coresight-dynamic-replicator for the programmable version, introduce a new name such as coresight-auto-funnel / coresight-concentrator for this non-programmable variant.
I'd still use the same non-prog platform driver code as you suggest - and change the driver source file name if you like, but reduce the amount of DT churn by avoiding a new name for an existing component.
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
Regards
Mike
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
On Tue, 13 Mar 2018 23:15:19 +0000 Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu, Kim
Hi Mike,
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 12 Mar 2018 11:28:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
Is it possible to keep everything equal (no new compatibles or other changes), and have functions like funnel_enable_hw, funnel_disable_hw do something like this instead?:
if (!drvdata->base) return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Right, but they needn't be separate: they both do the same thing: register their link operations' callbacks with coresight_register() after getting probed.
coresight-dynamic-replicator.c is probed as an AMBA bus driver, and coresight-replicator.c is an OF-based driver, but a single driver can match against different bus_types, and match tables, see e.g., drivers/tty/serial/amba-pl011.c.
The commit 620cf787c121 "coresight: replicator: Add Qualcomm CoreSight Replicator driver" that added the second 'dynamic' replicator driver doesn't explain why it needed to be a *separate* driver per se, and I'm suggesting here that it didn't - and more importantly that the coresight funnel driver doesn't need to be duplicated.
I'd be inclined to keep coresight-replicator for the non-programmable version, and perhaps in the same sense that we have a coresight-dynamic-replicator for the programmable version, introduce a new name such as coresight-auto-funnel / coresight-concentrator for this non-programmable variant.
I'd still use the same non-prog platform driver code as you suggest - and change the driver source file name if you like, but reduce the amount of DT churn by avoiding a new name for an existing component.
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
Yes, let's not introduce neither a new compatible, nor driver, and avoid unnecessary rename commits like 1c8859848dbb07 "coresight replicator: Cleanup programmable replicator naming" in the future.
Thanks,
Kim
On 13 March 2018 at 22:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 23:15:19 +0000 Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu, Kim
Hi Mike,
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote: > On Mon, 12 Mar 2018 11:28:33 -0600 > Mathieu Poirier mathieu.poirier@linaro.org wrote: > > Is it possible to keep everything equal (no new compatibles or other > changes), and have functions like funnel_enable_hw, funnel_disable_hw > do something like this instead?: > > if (!drvdata->base) > return;
Not that I'm aware of - a funnel device won't get probed if it doesn't have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Right, but they needn't be separate: they both do the same thing: register their link operations' callbacks with coresight_register() after getting probed.
coresight-dynamic-replicator.c is probed as an AMBA bus driver, and coresight-replicator.c is an OF-based driver, but a single driver can match against different bus_types, and match tables, see e.g., drivers/tty/serial/amba-pl011.c.
I hadn't seen an hybrid solution like that before - thanks for pointing it out. That should do the trick and does indeed have the advantage of keeping things in a single file and avoid the proliferation of compatible strings.
The commit 620cf787c121 "coresight: replicator: Add Qualcomm CoreSight Replicator driver" that added the second 'dynamic' replicator driver doesn't explain why it needed to be a *separate* driver per se, and I'm suggesting here that it didn't - and more importantly that the coresight funnel driver doesn't need to be duplicated.
I'd be inclined to keep coresight-replicator for the non-programmable version, and perhaps in the same sense that we have a coresight-dynamic-replicator for the programmable version, introduce a new name such as coresight-auto-funnel / coresight-concentrator for this non-programmable variant.
I'd still use the same non-prog platform driver code as you suggest - and change the driver source file name if you like, but reduce the amount of DT churn by avoiding a new name for an existing component.
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
Yes, let's not introduce neither a new compatible, nor driver, and avoid unnecessary rename commits like 1c8859848dbb07 "coresight replicator: Cleanup programmable replicator naming" in the future.
Thanks,
Kim
On 14/03/18 15:47, Mathieu Poirier wrote:
On 13 March 2018 at 22:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 23:15:19 +0000 Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu, Kim
Hi Mike,
On 13 March 2018 at 20:20, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 11:03:57 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 March 2018 at 10:06, Kim Phillips kim.phillips@arm.com wrote:
On Tue, 13 Mar 2018 09:40:14 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
> On 12 March 2018 at 13:02, Kim Phillips kim.phillips@arm.com wrote: >> On Mon, 12 Mar 2018 11:28:33 -0600 >> Mathieu Poirier mathieu.poirier@linaro.org wrote: >> >> Is it possible to keep everything equal (no new compatibles or other >> changes), and have functions like funnel_enable_hw, funnel_disable_hw >> do something like this instead?: >> >> if (!drvdata->base) >> return; > > Not that I'm aware of - a funnel device won't get probed if it doesn't > have a valid 'reg' property to poke at.
All a node needs to contain to get its driver probed is the compatible property. 'reg' is an optional property.
The initialisation code mandate that a valid address be given [1].
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L260
I see, thanks. So that's for an AMBA device, and the current funnel driver is an AMBA driver. Makes sense, but the current replicator driver - that supports non-configurable replicators by not including the "arm,primecell" compatible string - is a platform driver. Isn't it similarly possible to make the funnel driver a platform driver and match solely on the existing "arm,coresight-funnel" compatible? Only in the case of configurable funnels would it also manually register with the AMBA subsytem.
There are a pair of separate drivers for the replicators, one platform for non-programmable, one AMBA for dynamic (i.e. programmable).
Right, but they needn't be separate: they both do the same thing: register their link operations' callbacks with coresight_register() after getting probed.
coresight-dynamic-replicator.c is probed as an AMBA bus driver, and coresight-replicator.c is an OF-based driver, but a single driver can match against different bus_types, and match tables, see e.g., drivers/tty/serial/amba-pl011.c.
I hadn't seen an hybrid solution like that before - thanks for pointing it out. That should do the trick and does indeed have the advantage of keeping things in a single file and avoid the proliferation of compatible strings.
+1. FWIW, I am aware of another "case" of static funnel, waiting to make an entry in the drivers.
Suzuki
As to the "arm," prefix on all this - we need to double check that this is an actual arm variant - otherwise it will need to have the actual designer prefix instead. Haven't come across this variant before so need to check it is actually arm specified before claiming it as such.
So there are two ways funnels get into systems. One is from Arm's CoreSight 'toolkit' which provides all the components for the downstream trace network. In this toolkit, whether a funnel is programmable or not is a design-time option.
The other way is that some CPU clusters (not all) will have an internal non-programmable funnel to concentrate the output of all the ETMs in the cluster so the cluster only has to have one trace output bus rather than one for each core (or thread). In this case the funnel might be designed by whoever is designing the CPU and that might not be Arm. In fact, internally it might even be multiple levels of funnel.
So it's probably best to refer it in the most generic way possible. It's just a point in the trace topology at which ATB streams get merged.
Al IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-
Al Grant -
Kim Phillips -
Mathieu Poirier -
Mike Leach -
Suzuki K Poulose