Re: [RFC PATCH 0/6] net: ethernet: ti: cpsw: add MQPRIO and CBS Qdisc offload

2018-05-23 Thread Grygorii Strashko

Hi Ivan,

On 05/18/2018 04:15 PM, Ivan Khoronzhuk wrote:

This series adds MQPRIO and CBS Qdisc offload for TI cpsw driver.
It potentially can be used in audio video bridging (AVB) and time
sensitive networking (TSN).

Patchset was tested on AM572x EVM and BBB boards. Last patch from this
series adds detailed description of configuration with examples. For
consistency reasons, in role of talker and listener, tools from
patchset "TSN: Add qdisc based config interface for CBS" were used and
can be seen here: https://www.spinics.net/lists/netdev/msg460869.html

Based on net-next/master


Thanks a lot, it is great work.

In general I have no comments as of now, but I prefer to wait a bit (few 
weeks) for more comments and possible test reports.


If no comments, pls feel free to repost as final series.




Ivan Khoronzhuk (6):
   net: ethernet: ti: cpsw: use cpdma channels in backward order for txq
   net: ethernet: ti: cpdma: fit rated channels in backward order
   net: ethernet: ti: cpsw: add MQPRIO Qdisc offload
   net: ethernet: ti: cpsw: add CBS Qdisc offload
   net: ethernet: ti: cpsw: restore shaper configuration while down/up
   Documentation: networking: cpsw: add MQPRIO & CBS offload examples

  Documentation/networking/cpsw.txt   | 540 
  drivers/net/ethernet/ti/cpsw.c  | 364 +++-
  drivers/net/ethernet/ti/davinci_cpdma.c |  31 +-
  3 files changed, 913 insertions(+), 22 deletions(-)
  create mode 100644 Documentation/networking/cpsw.txt



--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume

2017-10-19 Thread Grygorii Strashko


On 10/19/2017 01:11 PM, Ulf Hansson wrote:
> On 19 October 2017 at 20:04, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>> On 19 October 2017 at 19:21, Grygorii Strashko <grygorii.stras...@ti.com> 
>> wrote:
>>>
>>>
>>> On 10/19/2017 03:33 AM, Ulf Hansson wrote:
>>>> On 18 October 2017 at 23:48, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
>>>>> On Wednesday, October 18, 2017 9:45:11 PM CEST Grygorii Strashko wrote:
>>>>>>
>>>>>> On 10/18/2017 09:11 AM, Ulf Hansson wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>>>>> That's the point. We know pm_runtime_force_* works nicely for the
>>>>>>>>> trivial middle-layer cases.
>>>>>>>>
>>>>>>>> In which cases the middle-layer callbacks don't exist, so it's just 
>>>>>>>> like
>>>>>>>> reusing driver callbacks directly. :-)
>>>>>>
>>>>>> I'd like to ask you clarify one point here and provide some info which I 
>>>>>> hope can be useful -
>>>>>> what's exactly means  "trivial middle-layer cases"?
>>>>>>
>>>>>> Is it when systems use "drivers/base/power/clock_ops.c - Generic clock
>>>>>> manipulation PM callbacks" as dev_pm_domain (arm davinci/keystone), or 
>>>>>> OMAP
>>>>>> device framework struct dev_pm_domain omap_device_pm_domain
>>>>>> (arm/mach-omap2/omap_device.c) or static const struct dev_pm_ops
>>>>>> tegra_aconnect_pm_ops?
>>>>>>
>>>>>> if yes all above have PM runtime callbacks.
>>>>>
>>>>> Trivial ones don't actually do anything meaningful in their PM callbacks.
>>>>>
>>>>> Things like the platform bus type, spi bus type, i2c bus type and similar.
>>>>>
>>>>> If the middle-layer callbacks manipulate devices in a significant way, 
>>>>> then
>>>>> they aren't trivial.
>>>>
>>>> I fully agree with Rafael's description above, but let me also clarify
>>>> one more thing.
>>>>
>>>> We have also been discussing PM domains as being trivial and
>>>> non-trivial. In some statements I even think the PM domain has been a
>>>> part the middle-layer terminology, which may have been a bit
>>>> confusing.
>>>>
>>>> In this regards as we consider genpd being a trivial PM domain, those
>>>> examples your bring up above is too me also examples of trivial PM
>>>> domains. Especially because they don't deal with wakeups, as that is
>>>> taken care of by the drivers, right!?
>>>
>>> Not directly, for example, omap device framework has noirq callback 
>>> implemented
>>> which forcibly disable all devices which are not PM runtime suspended.
>>> while doing this it calls drivers PM .runtime_suspend() which may return
>>> non 0 value and in this case device will be left enabled (powered) at 
>>> suspend for
>>> wake up purposes (see _od_suspend_noirq()).
>>>
>>
>> Yeah, I had that feeling that omap has some trickyness going on. :-)
>>
>> I sure that can be fixed in the omap PM domain, although
> 
> ...slipped with my fingers.. here is the rest of the reply...
> 
> ..of course that require us to use another way for drivers to signal
> to the omap PM domain that it needs to stay powered as to deal with
> wakeup.
> 
> I can have a look at that more closely, to see if it makes sense to change.
> 

Also, additional note here. some IPs are reused between OMAP/Davinci/Keystone,
OMAP PM domain have some code running at noirq time to dial with devices left
in PM runtime enabled state (OMAP PM runtime centric), while Davinci/Keystone 
haven't (clock_ops.c),
so pm_runtime_force_* API is actually possibility now to make the same driver 
work 
 on all these platforms. 

-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume

2017-10-19 Thread Grygorii Strashko


On 10/19/2017 03:33 AM, Ulf Hansson wrote:
> On 18 October 2017 at 23:48, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
>> On Wednesday, October 18, 2017 9:45:11 PM CEST Grygorii Strashko wrote:
>>>
>>> On 10/18/2017 09:11 AM, Ulf Hansson wrote:
>>
>> [...]
>>
>>>>>> That's the point. We know pm_runtime_force_* works nicely for the
>>>>>> trivial middle-layer cases.
>>>>>
>>>>> In which cases the middle-layer callbacks don't exist, so it's just like
>>>>> reusing driver callbacks directly. :-)
>>>
>>> I'd like to ask you clarify one point here and provide some info which I 
>>> hope can be useful -
>>> what's exactly means  "trivial middle-layer cases"?
>>>
>>> Is it when systems use "drivers/base/power/clock_ops.c - Generic clock
>>> manipulation PM callbacks" as dev_pm_domain (arm davinci/keystone), or OMAP
>>> device framework struct dev_pm_domain omap_device_pm_domain
>>> (arm/mach-omap2/omap_device.c) or static const struct dev_pm_ops
>>> tegra_aconnect_pm_ops?
>>>
>>> if yes all above have PM runtime callbacks.
>>
>> Trivial ones don't actually do anything meaningful in their PM callbacks.
>>
>> Things like the platform bus type, spi bus type, i2c bus type and similar.
>>
>> If the middle-layer callbacks manipulate devices in a significant way, then
>> they aren't trivial.
> 
> I fully agree with Rafael's description above, but let me also clarify
> one more thing.
> 
> We have also been discussing PM domains as being trivial and
> non-trivial. In some statements I even think the PM domain has been a
> part the middle-layer terminology, which may have been a bit
> confusing.
> 
> In this regards as we consider genpd being a trivial PM domain, those
> examples your bring up above is too me also examples of trivial PM
> domains. Especially because they don't deal with wakeups, as that is
> taken care of by the drivers, right!?

Not directly, for example, omap device framework has noirq callback implemented
which forcibly disable all devices which are not PM runtime suspended.
while doing this it calls drivers PM .runtime_suspend() which may return
non 0 value and in this case device will be left enabled (powered) at suspend 
for
wake up purposes (see _od_suspend_noirq()).


-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume

2017-10-18 Thread Grygorii Strashko


On 10/18/2017 09:11 AM, Ulf Hansson wrote:
> [...]
> 
>>>
>>> The reason why pm_runtime_force_* needs to respects the hierarchy of
>>> the RPM callbacks, is because otherwise it can't safely update the
>>> runtime PM status of the device.
>>
>> I'm not sure I follow this requirement.  Why is that so?
> 
> If the PM domain controls some resources for the device in its RPM
> callbacks and the driver controls some other resources in its RPM
> callbacks - then these resources needs to be managed together.
> 
> This follows the behavior of when a regular call to
> pm_runtime_get|put(), triggers the RPM callbacks to be invoked.
> 
>>
>>> And updating the runtime PM status of
>>> the device is required to manage the optimized behavior during system
>>> resume (avoiding to unnecessary resume devices).
>>
>> Well, OK.  The runtime PM status of the device after system resume should
>> better reflect its physical state.
>>
>> [The physical state of the device may not be under the control of the
>> kernel in some cases, like in S3 resume on some systems that reset
>> devices in the firmware and so on, but let's set that aside.]
>>
>> However, for the runtime PM status of the device may still reflect its state
>> if, say, a ->resume_early of the middle layer is called during resume along
>> with a driver's ->runtime_resume.  That still can produce the right state
>> of the device and all depends on the middle layer.
>>
>> On the other hand, as I said before, using a middle-layer ->runtime_suspend
>> during a system sleep transition may be outright incorrect, say if device
>> wakeup settings need to be adjusted by the middle layer (which is the
>> case for some of them).
>>
>> Of course, if the middle layer expects the driver to point its
>> system-wide PM callbacks to pm_runtime_force_*, then that's how it goes,
>> but the drivers working with this particular middle layer generally
>> won't work with other middle layers and may interact incorrectly
>> with parents and/or children using the other middle layers.
>>
>> I guess the problem boils down to having a common set of expectations
>> on the driver side and on the middle layer side allowing different
>> combinations of these to work together.
> 
> Yes!
> 
>>
>>> Besides the AMBA case, I also realized that we are dealing with PM
>>> clocks in the genpd case. For this, genpd relies on the that runtime
>>> PM status of the device properly reflects the state of the HW, during
>>> system-wide PM.
>>>
>>> In other words, if the driver would change the runtime PM status of
>>> the device, without respecting the hierarchy of the runtime PM
>>> callbacks, it would lead to that genpd starts taking wrong decisions
>>> while managing the PM clocks during system-wide PM. So in case you
>>> intend to change pm_runtime_force_* this needs to be addressed too.
>>
>> I've just looked at the genpd code and quite frankly I'm not sure how this
>> works, but I'll figure this out. :-)
> 
> You may think of it as genpd's RPM callback controls some device
> clocks, while the driver control some other device resources (pinctrl
> for example) from its RPM callback.
> 
> These resources needs to managed together, similar to as I described above.
> 
> [...]
> 
>>> Absolutely agree about the different wake-up settings. However, these
>>> issues can be addressed also when using pm_runtime_force_*, at least
>>> in general, but then not for PCI.
>>
>> Well, not for the ACPI PM domain too.
>>
>> In general, not if the wakeup settings are adjusted by the middle layer.
> 
> Correct!
> 
> To use pm_runtime_force* for these cases, one would need some
> additional information exchange between the driver and the
> middle-layer.
> 
>>
>>> Regarding hibernation, honestly that's not really my area of
>>> expertise. Although, I assume the middle-layer and driver can treat
>>> that as a separate case, so if it's not suitable to use
>>> pm_runtime_force* for that case, then they shouldn't do it.
>>
>> Well, agreed.
>>
>> In some simple cases, though, driver callbacks can be reused for hibernation
>> too, so it would be good to have a common way to do that too, IMO.
> 
> Okay, that makes sense!
> 
>>

 Also, quite so often other middle layers interact with PCI directly or
 indirectly (eg. a platform device may be a child or a consumer of a PCI
 device) and some optimizations need to take that into account (eg. parents
 generally need to be accessible when their childres are resumed and so on).
>>>
>>> A device's parent becomes informed when changing the runtime PM status
>>> of the device via pm_runtime_force_suspend|resume(), as those calls
>>> pm_runtime_set_suspended|active().
>>
>> This requires the parent driver or middle layer to look at the reference
>> counter and understand it the same way as pm_runtime_force_*.
>>
>>> In case that isn't that sufficient, what else is needed? Perhaps you can
>>> point me to an example so I can understand better?
>>
>> Say you want to leave the