> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/err.h>
> > +#include <linux/printk.h>
> > +#include <linux/modem_shm/modem.h>
> > +
> > +static struct class *modem_class;
>
> What's wrong with a bus_type instead?
Can I know the advantage of using bus_type over class?
>
> > +static int __modem_is_requested(struct device *dev, void *data) {
> > + struct modem_desc *mdesc = (struct modem_desc *)data;
> > +
> > + if (!mdesc->mclients) {
> > + printk(KERN_ERR "modem_access: modem description is
> NULL\n");
> > + return 0;
> > + }
> > + return atomic_read(&mdesc->mclients->cnt);
> > +}
> > +
> > +int modem_is_requested(struct modem_desc *mdesc) {
> > + return class_for_each_device(modem_class, NULL, (void *)mdesc,
> > +__modem_is_requested); }
>
> Where is the documentation for your public api functions like this?
Sure will include this in the next patchset.
>
> > +
> > +int modem_release(struct modem_desc *mdesc) {
> > + if (!mdesc->release)
> > + return -EFAULT;
> > +
> > + if (modem_is_requested(mdesc)) {
> > + atomic_dec(&mdesc->mclients->cnt);
> > + if (atomic_read(&mdesc->use_cnt) == 1) {
> > + mdesc->release(mdesc);
> > + atomic_dec(&mdesc->use_cnt);
> > + }
>
> Eeek, why aren't you using the built-in reference counting that the struct
> device provided to you, and instead are rolling your own? This happens in
> many places, why?
My usage of counters over here is for each modem there are many clients.
Each of the clients will have a ref to modem_desc. Each of them use this for
requesting and releasing the modem. One counter for tracking the request
and release for each client which is done by variable 'cnt' in struct clients.
The counter use_cnt is used for tracking the modem request/release irrespective
of the clients and counter cli_cnt is used for restricting the modem_get to
the no of clients defined in no_clients.
So totally 3 counter one for restricting the usage of modem_get by clients,
second for restricting modem request/release at top level, and 3rd for
restricting modem release/request for per client per modem basis.
Can you let me know if the same can be achieved by using built-in ref
counting?
Thanks and Regards,
Arun R Murthy
------------------
On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY <[email protected]> wrote:
>> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
>> > +#include <linux/module.h>
>> > +#include <linux/slab.h>
>> > +#include <linux/err.h>
>> > +#include <linux/printk.h>
>> > +#include <linux/modem_shm/modem.h>
>> > +
>> > +static struct class *modem_class;
>>
>> What's wrong with a bus_type instead?
>
> Can I know the advantage of using bus_type over class?
>
>>
>> > +static int __modem_is_requested(struct device *dev, void *data) {
>> > + struct modem_desc *mdesc = (struct modem_desc *)data;
>> > +
>> > + if (!mdesc->mclients) {
>> > + printk(KERN_ERR "modem_access: modem description is
>> NULL\n");
>> > + return 0;
>> > + }
>> > + return atomic_read(&mdesc->mclients->cnt);
>> > +}
>> > +
>> > +int modem_is_requested(struct modem_desc *mdesc) {
>> > + return class_for_each_device(modem_class, NULL, (void *)mdesc,
>> > +__modem_is_requested); }
>>
>> Where is the documentation for your public api functions like this?
>
> Sure will include this in the next patchset.
>
>>
>> > +
>> > +int modem_release(struct modem_desc *mdesc) {
>> > + if (!mdesc->release)
>> > + return -EFAULT;
>> > +
>> > + if (modem_is_requested(mdesc)) {
>> > + atomic_dec(&mdesc->mclients->cnt);
>> > + if (atomic_read(&mdesc->use_cnt) == 1) {
>> > + mdesc->release(mdesc);
>> > + atomic_dec(&mdesc->use_cnt);
>> > + }
>>
>> Eeek, why aren't you using the built-in reference counting that the struct
>> device provided to you, and instead are rolling your own? This happens in
>> many places, why?
>
> My usage of counters over here is for each modem there are many clients.
> Each of the clients will have a ref to modem_desc. Each of them use this for
> requesting and releasing the modem. One counter for tracking the request
> and release for each client which is done by variable 'cnt' in struct clients.
> The counter use_cnt is used for tracking the modem request/release irrespective
> of the clients and counter cli_cnt is used for restricting the modem_get to
> the no of clients defined in no_clients.
>
> So totally 3 counter one for restricting the usage of modem_get by clients,
> second for restricting modem request/release at top level, and 3rd for
> restricting modem release/request for per client per modem basis.
>
> Can you let me know if the same can be achieved by using built-in ref
> counting?
Is this your model:
You have a modem device which can be requested by many clients.This clients
can register for a particular service which this modem provides and then after
that if it client doesn't need this service then it will call un-register.
This can happen for many clients.
So what you need is a way to track clients and once no client is in picture, you
want to de-allocate all the memory and resource associated with modem device.
If this is your model then read on otherwise please skip.
What you can do is this:
On each modem_register
list_add(&modm_dev->entry, &modm_dev_list);
and once you de-register, remove the device from the modem_dev_list.
Have this in your modem_register function
modem->dev->release = modem_dev_release;
This will be called once all the device references have been released
and you need to remove all the memory/resources associated with your
modem device.So you will do the final cleanup
modem_cleanup(edev, true); //this will be "false" when the client just does
the modem_unregister.
Something as below:
void modem_dev_unregister(struct modem_dev *edev)
{
modem_cleanup(edev, false);
}
static void modem_dev_release(struct device *dev)
{
struct modem_dev *edev = (struct modem_dev *) dev_get_drvdata(dev);
modem_cleanup(edev, true);
}
static void modem_cleanup(struct modem_dev *edev, bool skip)
{
mutex_lock(&modem_dev_list_lock);
list_del(&modem->entry);
mutex_unlock(&modem_dev_list_lock);
if (!skip && get_device(modem->dev)) {
//do the cleanup here
}
device_unregister(modem->dev);
put_device(modem->dev);
}
kfree(modem->dev);
}
>
> Thanks and Regards,
> Arun R Murthy
> ------------------
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
> On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY
> <[email protected]> wrote:
> >> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> >> > +#include <linux/module.h>
> >> > +#include <linux/slab.h>
> >> > +#include <linux/err.h>
> >> > +#include <linux/printk.h>
> >> > +#include <linux/modem_shm/modem.h>
> >> > +
> >> > +static struct class *modem_class;
> >>
> >> What's wrong with a bus_type instead?
> >
> > Can I know the advantage of using bus_type over class?
> >
> >>
> >> > +static int __modem_is_requested(struct device *dev, void *data) {
> >> > + struct modem_desc *mdesc = (struct modem_desc *)data;
> >> > +
> >> > + if (!mdesc->mclients) {
> >> > + printk(KERN_ERR "modem_access: modem description is
> >> NULL\n");
> >> > + return 0;
> >> > + }
> >> > + return atomic_read(&mdesc->mclients->cnt);
> >> > +}
> >> > +
> >> > +int modem_is_requested(struct modem_desc *mdesc) {
> >> > + return class_for_each_device(modem_class, NULL, (void *)mdesc,
> >> > +__modem_is_requested); }
> >>
> >> Where is the documentation for your public api functions like this?
> >
> > Sure will include this in the next patchset.
> >
> >>
> >> > +
> >> > +int modem_release(struct modem_desc *mdesc) {
> >> > + if (!mdesc->release)
> >> > + return -EFAULT;
> >> > +
> >> > + if (modem_is_requested(mdesc)) {
> >> > + atomic_dec(&mdesc->mclients->cnt);
> >> > + if (atomic_read(&mdesc->use_cnt) == 1) {
> >> > + mdesc->release(mdesc);
> >> > + atomic_dec(&mdesc->use_cnt);
> >> > + }
> >>
> >> Eeek, why aren't you using the built-in reference counting that the
> >> struct device provided to you, and instead are rolling your own?
> >> This happens in many places, why?
> >
> > My usage of counters over here is for each modem there are many clients.
> > Each of the clients will have a ref to modem_desc. Each of them use
> > this for requesting and releasing the modem. One counter for tracking
> > the request and release for each client which is done by variable 'cnt' in
> struct clients.
> > The counter use_cnt is used for tracking the modem request/release
> > irrespective of the clients and counter cli_cnt is used for
> > restricting the modem_get to the no of clients defined in no_clients.
> >
> > So totally 3 counter one for restricting the usage of modem_get by
> > clients, second for restricting modem request/release at top level,
> > and 3rd for restricting modem release/request for per client per modem
> basis.
> >
> > Can you let me know if the same can be achieved by using built-in ref
> > counting?
> Is this your model:
> You have a modem device which can be requested by many clients.This
> clients can register for a particular service which this modem provides and
> then after that if it client doesn't need this service then it will call un-register.
Let me correct a bit over here:
There are many clients, yes correct but the operations performed are only two,
i.e modem request and modem release. This is something like waking up the
modem and let modem to sleep.
The traffic of this request and release is too high.
So irrespective of the requests/releases made to the MAF framework, the MAF
should perform the operation request/release only once.
So each and every time handling list consumes time.
Let me brief the context, this is a single chip modem and ape, basically used in
mobile, tablets etc. So the traffic in ape-modem communication is too high and
also time critical. If it bound to exceed the time, or delay might end up in buffer
full. That’s the reason I have made it as simple as possible.
Thanks and Regards,
Arun R Murthy
------------------
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Mon, Oct 1, 2012 at 12:27 PM, Arun MURTHY <[email protected]> wrote:
>> On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY
>> <[email protected]> wrote:
>> >> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
>> >> > +#include <linux/module.h>
>> >> > +#include <linux/slab.h>
>> >> > +#include <linux/err.h>
>> >> > +#include <linux/printk.h>
>> >> > +#include <linux/modem_shm/modem.h>
>> >> > +
>> >> > +static struct class *modem_class;
>> >>
>> >> What's wrong with a bus_type instead?
>> >
>> > Can I know the advantage of using bus_type over class?
>> >
>> >>
>> >> > +static int __modem_is_requested(struct device *dev, void *data) {
>> >> > + struct modem_desc *mdesc = (struct modem_desc *)data;
>> >> > +
>> >> > + if (!mdesc->mclients) {
>> >> > + printk(KERN_ERR "modem_access: modem description is
>> >> NULL\n");
>> >> > + return 0;
>> >> > + }
>> >> > + return atomic_read(&mdesc->mclients->cnt);
>> >> > +}
>> >> > +
>> >> > +int modem_is_requested(struct modem_desc *mdesc) {
>> >> > + return class_for_each_device(modem_class, NULL, (void *)mdesc,
>> >> > +__modem_is_requested); }
>> >>
>> >> Where is the documentation for your public api functions like this?
>> >
>> > Sure will include this in the next patchset.
>> >
>> >>
>> >> > +
>> >> > +int modem_release(struct modem_desc *mdesc) {
>> >> > + if (!mdesc->release)
>> >> > + return -EFAULT;
>> >> > +
>> >> > + if (modem_is_requested(mdesc)) {
>> >> > + atomic_dec(&mdesc->mclients->cnt);
>> >> > + if (atomic_read(&mdesc->use_cnt) == 1) {
>> >> > + mdesc->release(mdesc);
>> >> > + atomic_dec(&mdesc->use_cnt);
>> >> > + }
>> >>
>> >> Eeek, why aren't you using the built-in reference counting that the
>> >> struct device provided to you, and instead are rolling your own?
>> >> This happens in many places, why?
>> >
>> > My usage of counters over here is for each modem there are many clients.
>> > Each of the clients will have a ref to modem_desc. Each of them use
>> > this for requesting and releasing the modem. One counter for tracking
>> > the request and release for each client which is done by variable 'cnt' in
>> struct clients.
>> > The counter use_cnt is used for tracking the modem request/release
>> > irrespective of the clients and counter cli_cnt is used for
>> > restricting the modem_get to the no of clients defined in no_clients.
>> >
>> > So totally 3 counter one for restricting the usage of modem_get by
>> > clients, second for restricting modem request/release at top level,
>> > and 3rd for restricting modem release/request for per client per modem
>> basis.
>> >
>> > Can you let me know if the same can be achieved by using built-in ref
>> > counting?
>> Is this your model:
>> You have a modem device which can be requested by many clients.This
>> clients can register for a particular service which this modem provides and
>> then after that if it client doesn't need this service then it will call un-register.
>
> Let me correct a bit over here:
> There are many clients, yes correct but the operations performed are only two,
> i.e modem request and modem release. This is something like waking up the
> modem and let modem to sleep.
> The traffic of this request and release is too high.
>
> So irrespective of the requests/releases made to the MAF framework, the MAF
> should perform the operation request/release only once.
>
> So each and every time handling list consumes time.
> Let me brief the context, this is a single chip modem and ape, basically used in
> mobile, tablets etc. So the traffic in ape-modem communication is too high and
> also time critical. If it bound to exceed the time, or delay might end up in buffer
> full. That’s the reason I have made it as simple as possible.
So let me put it this way:
Modem Client1 Client2 Client3 Client4
State turn-on request
State no-state-change request
State no-state-change request
State no-state-change
request
State no-state-change release
State no-state-change release
State no-state-change release
State turn-off
release
So eventhough all the clients have requested the modem it is being
turned on only once and when all of them have released then it will be
turned-off.
In this case it makes sense to use the atomic variables to track the requests
and release but what will happen to sysfs incase the device is released when the
last call to release has come from client4?Won't it lead to kernel panic or some
unwanted behaviour?
>
> Thanks and Regards,
> Arun R Murthy
> ------------------
> >> >> > +int modem_release(struct modem_desc *mdesc) {
> >> >> > + if (!mdesc->release)
> >> >> > + return -EFAULT;
> >> >> > +
> >> >> > + if (modem_is_requested(mdesc)) {
> >> >> > + atomic_dec(&mdesc->mclients->cnt);
> >> >> > + if (atomic_read(&mdesc->use_cnt) == 1) {
> >> >> > + mdesc->release(mdesc);
> >> >> > + atomic_dec(&mdesc->use_cnt);
> >> >> > + }
> >> >>
> >> >> Eeek, why aren't you using the built-in reference counting that
> >> >> the struct device provided to you, and instead are rolling your own?
> >> >> This happens in many places, why?
> >> >
> >> > My usage of counters over here is for each modem there are many
> clients.
> >> > Each of the clients will have a ref to modem_desc. Each of them use
> >> > this for requesting and releasing the modem. One counter for
> >> > tracking the request and release for each client which is done by
> >> > variable 'cnt' in
> >> struct clients.
> >> > The counter use_cnt is used for tracking the modem request/release
> >> > irrespective of the clients and counter cli_cnt is used for
> >> > restricting the modem_get to the no of clients defined in no_clients.
> >> >
> >> > So totally 3 counter one for restricting the usage of modem_get by
> >> > clients, second for restricting modem request/release at top level,
> >> > and 3rd for restricting modem release/request for per client per
> >> > modem
> >> basis.
> >> >
> >> > Can you let me know if the same can be achieved by using built-in
> >> > ref counting?
> >> Is this your model:
> >> You have a modem device which can be requested by many clients.This
> >> clients can register for a particular service which this modem
> >> provides and then after that if it client doesn't need this service then it will
> call un-register.
> >
> > Let me correct a bit over here:
> > There are many clients, yes correct but the operations performed are
> > only two, i.e modem request and modem release. This is something like
> > waking up the modem and let modem to sleep.
> > The traffic of this request and release is too high.
> >
> > So irrespective of the requests/releases made to the MAF framework,
> > the MAF should perform the operation request/release only once.
> >
> > So each and every time handling list consumes time.
> > Let me brief the context, this is a single chip modem and ape,
> > basically used in mobile, tablets etc. So the traffic in ape-modem
> > communication is too high and also time critical. If it bound to
> > exceed the time, or delay might end up in buffer full. That’s the reason I
> have made it as simple as possible.
>
> So let me put it this way:
> Modem Client1 Client2 Client3 Client4
> State turn-on request
> State no-state-change request
> State no-state-change request
> State no-state-change
> request
> State no-state-change release
> State no-state-change release
> State no-state-change release
> State turn-off
> release
>
> So eventhough all the clients have requested the modem it is being turned
> on only once and when all of them have released then it will be turned-off.
>
> In this case it makes sense to use the atomic variables to track the requests
> and release but what will happen to sysfs incase the device is released when
> the last call to release has come from client4?Won't it lead to kernel panic or
> some unwanted behaviour?
>
Yes, you are right, so will add a check in modem_put and modem_unregister
to check if modem is requested and if so will release first and then go ahead.
But actually clients are not suppose to call modem_put/modem_unregister
unless modem is released but yes, in MAF we can take this precaution.
Thanks and Regards,
Arun R Murthy
-----------------
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > > +#include <linux/module.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/err.h>
> > > +#include <linux/printk.h>
> > > +#include <linux/modem_shm/modem.h>
> > > +
> > > +static struct class *modem_class;
> >
> > What's wrong with a bus_type instead?
>
> Can I know the advantage of using bus_type over class?
You have devices living on a bus, and it's much more descriptive than a
class (which we are going to eventually get rid of one of these
days...).
Might I ask why you choose a class over a bus_type?
> > > +int modem_release(struct modem_desc *mdesc) {
> > > + if (!mdesc->release)
> > > + return -EFAULT;
> > > +
> > > + if (modem_is_requested(mdesc)) {
> > > + atomic_dec(&mdesc->mclients->cnt);
> > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> > > + mdesc->release(mdesc);
> > > + atomic_dec(&mdesc->use_cnt);
> > > + }
> >
> > Eeek, why aren't you using the built-in reference counting that the struct
> > device provided to you, and instead are rolling your own? This happens in
> > many places, why?
>
> My usage of counters over here is for each modem there are many clients.
> Each of the clients will have a ref to modem_desc. Each of them use this for
> requesting and releasing the modem. One counter for tracking the request
> and release for each client which is done by variable 'cnt' in struct clients.
> The counter use_cnt is used for tracking the modem request/release irrespective
> of the clients and counter cli_cnt is used for restricting the modem_get to
> the no of clients defined in no_clients.
>
> So totally 3 counter one for restricting the usage of modem_get by clients,
> second for restricting modem request/release at top level, and 3rd for
> restricting modem release/request for per client per modem basis.
>
> Can you let me know if the same can be achieved by using built-in ref
> counting?
Yes, because you don't need all of those different levels, just stick
with one and you should be fine. :)
thanks,
greg k-h
> On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > > > +#include <linux/module.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/printk.h>
> > > > +#include <linux/modem_shm/modem.h>
> > > > +
> > > > +static struct class *modem_class;
> > >
> > > What's wrong with a bus_type instead?
> >
> > Can I know the advantage of using bus_type over class?
>
> You have devices living on a bus, and it's much more descriptive than a class
> (which we are going to eventually get rid of one of these days...).
>
> Might I ask why you choose a class over a bus_type?
Basically my requirement is to create a central entity for accessing and releasing
modem from APE. Since this is done by different clients the central entity should
be able to handle the request and play safely, since this has more affect in
system suspend and deep sleep. Using class helps me in achieving this and
also create an entry to user space which can be used in the later parts. Moreover
this not something like a bus or so, so I didn't use bus instead went with a
simple class approach.
>
> > > > +int modem_release(struct modem_desc *mdesc) {
> > > > + if (!mdesc->release)
> > > > + return -EFAULT;
> > > > +
> > > > + if (modem_is_requested(mdesc)) {
> > > > + atomic_dec(&mdesc->mclients->cnt);
> > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> > > > + mdesc->release(mdesc);
> > > > + atomic_dec(&mdesc->use_cnt);
> > > > + }
> > >
> > > Eeek, why aren't you using the built-in reference counting that the
> > > struct device provided to you, and instead are rolling your own?
> > > This happens in many places, why?
> >
> > My usage of counters over here is for each modem there are many clients.
> > Each of the clients will have a ref to modem_desc. Each of them use
> > this for requesting and releasing the modem. One counter for tracking
> > the request and release for each client which is done by variable 'cnt' in
> struct clients.
> > The counter use_cnt is used for tracking the modem request/release
> > irrespective of the clients and counter cli_cnt is used for
> > restricting the modem_get to the no of clients defined in no_clients.
> >
> > So totally 3 counter one for restricting the usage of modem_get by
> > clients, second for restricting modem request/release at top level,
> > and 3rd for restricting modem release/request for per client per modem
> basis.
> >
> > Can you let me know if the same can be achieved by using built-in ref
> > counting?
>
> Yes, because you don't need all of those different levels, just stick with one
> and you should be fine. :)
>
No, checks at all these levels are required, I have briefed out the need also.
This will have effect on system power management, i.e suspend and deep
sleep.
We restrict that the drivers should request modem only once and release
only once, but we cannot rely on the clients hence a check for the same has
to be done in the MAF. Also the no of clients should be defined and hence a
check for the same is done in MAF. Apart from all these the requests coming
from all the clients is to be accumulated and based on that modem release
or access should be performed, hence so.
Thanks and Regards,
Arun R Murthy
-------------------
On Wed, Oct 3, 2012 at 9:24 AM, Arun MURTHY <[email protected]> wrote:
>> On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
>> > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
>> > > > +#include <linux/module.h>
>> > > > +#include <linux/slab.h>
>> > > > +#include <linux/err.h>
>> > > > +#include <linux/printk.h>
>> > > > +#include <linux/modem_shm/modem.h>
>> > > > +
>> > > > +static struct class *modem_class;
>> > >
>> > > What's wrong with a bus_type instead?
>> >
>> > Can I know the advantage of using bus_type over class?
>>
>> You have devices living on a bus, and it's much more descriptive than a class
>> (which we are going to eventually get rid of one of these days...).
>>
>> Might I ask why you choose a class over a bus_type?
>
> Basically my requirement is to create a central entity for accessing and releasing
> modem from APE. Since this is done by different clients the central entity should
> be able to handle the request and play safely, since this has more affect in
> system suspend and deep sleep. Using class helps me in achieving this and
> also create an entry to user space which can be used in the later parts. Moreover
You can have that same mechanism work for bus_type as well.
> this not something like a bus or so, so I didn't use bus instead went with a
> simple class approach.
>
>>
>> > > > +int modem_release(struct modem_desc *mdesc) {
>> > > > + if (!mdesc->release)
>> > > > + return -EFAULT;
>> > > > +
>> > > > + if (modem_is_requested(mdesc)) {
>> > > > + atomic_dec(&mdesc->mclients->cnt);
>> > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
>> > > > + mdesc->release(mdesc);
>> > > > + atomic_dec(&mdesc->use_cnt);
>> > > > + }
>> > >
>> > > Eeek, why aren't you using the built-in reference counting that the
>> > > struct device provided to you, and instead are rolling your own?
>> > > This happens in many places, why?
>> >
>> > My usage of counters over here is for each modem there are many clients.
>> > Each of the clients will have a ref to modem_desc. Each of them use
>> > this for requesting and releasing the modem. One counter for tracking
>> > the request and release for each client which is done by variable 'cnt' in
>> struct clients.
>> > The counter use_cnt is used for tracking the modem request/release
>> > irrespective of the clients and counter cli_cnt is used for
>> > restricting the modem_get to the no of clients defined in no_clients.
>> >
>> > So totally 3 counter one for restricting the usage of modem_get by
>> > clients, second for restricting modem request/release at top level,
>> > and 3rd for restricting modem release/request for per client per modem
>> basis.
>> >
>> > Can you let me know if the same can be achieved by using built-in ref
>> > counting?
>>
>> Yes, because you don't need all of those different levels, just stick with one
>> and you should be fine. :)
>>
>
> No, checks at all these levels are required, I have briefed out the need also.
> This will have effect on system power management, i.e suspend and deep
> sleep.
> We restrict that the drivers should request modem only once and release
> only once, but we cannot rely on the clients hence a check for the same has
> to be done in the MAF. Also the no of clients should be defined and hence a
> check for the same is done in MAF. Apart from all these the requests coming
> from all the clients is to be accumulated and based on that modem release
> or access should be performed, hence so.
I think best way to deal with this is:
Define a new bus type and have your clients call the bus exposed functionality
when ever they need a service.So in your case it would be request and release
only AND when all of your clients have released the bus then you can do the
cleanup i.e. switch off the modem and on added advantage of making it a bus_type
would be that you can do the reference counting in your bus driver.
Designing is not my forte but I feel this way you can solve the problem at hand.
Please feel free to correct me.I would really appreciate it.
>
> Thanks and Regards,
> Arun R Murthy
> -------------------
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
> On Wed, Oct 3, 2012 at 9:24 AM, Arun MURTHY
> <[email protected]> wrote:
> >> On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> >> > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> >> > > > +#include <linux/module.h>
> >> > > > +#include <linux/slab.h>
> >> > > > +#include <linux/err.h>
> >> > > > +#include <linux/printk.h>
> >> > > > +#include <linux/modem_shm/modem.h>
> >> > > > +
> >> > > > +static struct class *modem_class;
> >> > >
> >> > > What's wrong with a bus_type instead?
> >> >
> >> > Can I know the advantage of using bus_type over class?
> >>
> >> You have devices living on a bus, and it's much more descriptive than
> >> a class (which we are going to eventually get rid of one of these days...).
> >>
> >> Might I ask why you choose a class over a bus_type?
> >
> > Basically my requirement is to create a central entity for accessing
> > and releasing modem from APE. Since this is done by different clients
> > the central entity should be able to handle the request and play
> > safely, since this has more affect in system suspend and deep sleep.
> > Using class helps me in achieving this and also create an entry to
> > user space which can be used in the later parts. Moreover
> You can have that same mechanism work for bus_type as well.
> > this not something like a bus or so, so I didn't use bus instead went
> > with a simple class approach.
> >
> >>
> >> > > > +int modem_release(struct modem_desc *mdesc) {
> >> > > > + if (!mdesc->release)
> >> > > > + return -EFAULT;
> >> > > > +
> >> > > > + if (modem_is_requested(mdesc)) {
> >> > > > + atomic_dec(&mdesc->mclients->cnt);
> >> > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> >> > > > + mdesc->release(mdesc);
> >> > > > + atomic_dec(&mdesc->use_cnt);
> >> > > > + }
> >> > >
> >> > > Eeek, why aren't you using the built-in reference counting that
> >> > > the struct device provided to you, and instead are rolling your own?
> >> > > This happens in many places, why?
> >> >
> >> > My usage of counters over here is for each modem there are many
> clients.
> >> > Each of the clients will have a ref to modem_desc. Each of them use
> >> > this for requesting and releasing the modem. One counter for
> >> > tracking the request and release for each client which is done by
> >> > variable 'cnt' in
> >> struct clients.
> >> > The counter use_cnt is used for tracking the modem request/release
> >> > irrespective of the clients and counter cli_cnt is used for
> >> > restricting the modem_get to the no of clients defined in no_clients.
> >> >
> >> > So totally 3 counter one for restricting the usage of modem_get by
> >> > clients, second for restricting modem request/release at top level,
> >> > and 3rd for restricting modem release/request for per client per
> >> > modem
> >> basis.
> >> >
> >> > Can you let me know if the same can be achieved by using built-in
> >> > ref counting?
> >>
> >> Yes, because you don't need all of those different levels, just stick
> >> with one and you should be fine. :)
> >>
> >
> > No, checks at all these levels are required, I have briefed out the need also.
> > This will have effect on system power management, i.e suspend and deep
> > sleep.
> > We restrict that the drivers should request modem only once and
> > release only once, but we cannot rely on the clients hence a check for
> > the same has to be done in the MAF. Also the no of clients should be
> > defined and hence a check for the same is done in MAF. Apart from all
> > these the requests coming from all the clients is to be accumulated
> > and based on that modem release or access should be performed, hence
> so.
> I think best way to deal with this is:
> Define a new bus type and have your clients call the bus exposed
> functionality when ever they need a service.So in your case it would be
> request and release only AND when all of your clients have released the bus
> then you can do the cleanup i.e. switch off the modem and on added
> advantage of making it a bus_type would be that you can do the reference
> counting in your bus driver.
>
> Designing is not my forte but I feel this way you can solve the problem at
> hand.
> Please feel free to correct me.I would really appreciate it.
At the very first look itself this MAF is not a bus by its technical meaning, so
why to use bus_type is the point that I have.
Thanks and Regards,
Arun R Murthy
------------------
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Wed, Oct 03, 2012 at 05:54:08AM +0200, Arun MURTHY wrote:
> > On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> > > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/slab.h>
> > > > > +#include <linux/err.h>
> > > > > +#include <linux/printk.h>
> > > > > +#include <linux/modem_shm/modem.h>
> > > > > +
> > > > > +static struct class *modem_class;
> > > >
> > > > What's wrong with a bus_type instead?
> > >
> > > Can I know the advantage of using bus_type over class?
> >
> > You have devices living on a bus, and it's much more descriptive than a class
> > (which we are going to eventually get rid of one of these days...).
> >
> > Might I ask why you choose a class over a bus_type?
>
> Basically my requirement is to create a central entity for accessing and releasing
> modem from APE.
What is an "APE"?
And what do you mean by "accessing" and "releasing"?
> Since this is done by different clients the central entity should
> be able to handle the request and play safely, since this has more affect in
> system suspend and deep sleep. Using class helps me in achieving this
> and also create an entry to user space which can be used in the later
> parts. Moreover this not something like a bus or so, so I didn't use
> bus instead went with a simple class approach.
But as you have devices that are "binding" to this "controller", a bus
might make more sense, right?
I don't see how a class helps out for you here more than anything else,
what are you expecting from the class interface? You aren't using the
reference counting logic it provides, so why use it at all?
Actually, why use the driver core at all in the first place if you
aren't needing the devices to show up in sysfs (as you don't have a
device, you are just a mediator)?
> > > > > +int modem_release(struct modem_desc *mdesc) {
> > > > > + if (!mdesc->release)
> > > > > + return -EFAULT;
> > > > > +
> > > > > + if (modem_is_requested(mdesc)) {
> > > > > + atomic_dec(&mdesc->mclients->cnt);
> > > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> > > > > + mdesc->release(mdesc);
> > > > > + atomic_dec(&mdesc->use_cnt);
> > > > > + }
> > > >
> > > > Eeek, why aren't you using the built-in reference counting that the
> > > > struct device provided to you, and instead are rolling your own?
> > > > This happens in many places, why?
> > >
> > > My usage of counters over here is for each modem there are many clients.
> > > Each of the clients will have a ref to modem_desc. Each of them use
> > > this for requesting and releasing the modem. One counter for tracking
> > > the request and release for each client which is done by variable 'cnt' in
> > struct clients.
> > > The counter use_cnt is used for tracking the modem request/release
> > > irrespective of the clients and counter cli_cnt is used for
> > > restricting the modem_get to the no of clients defined in no_clients.
> > >
> > > So totally 3 counter one for restricting the usage of modem_get by
> > > clients, second for restricting modem request/release at top level,
> > > and 3rd for restricting modem release/request for per client per modem
> > basis.
> > >
> > > Can you let me know if the same can be achieved by using built-in ref
> > > counting?
> >
> > Yes, because you don't need all of those different levels, just stick with one
> > and you should be fine. :)
> >
>
> No, checks at all these levels are required, I have briefed out the need also.
I still don't understand, sorry.
> This will have effect on system power management, i.e suspend and deep
> sleep.
How does power management matter? If you tie into the driver model
properly, power management comes "for free" so you don't have to do
anything special about it. Why not use that logic instead of trying to
roll your own?
> We restrict that the drivers should request modem only once and release
> only once, but we cannot rely on the clients hence a check for the same has
> to be done in the MAF.
You can't rely on the clients to do what? And why can't you rely on
them? What is going to happen? Who is a "client" here? Other kernel
code?
I really don't understand your model at all as to what you are trying to
mediate and manage here, sorry. I suggest writing it all up as your
first patch (documentation is good), so that we can properly review your
implementation and not argue about how to implement something that I
honestly don't understand.
> Also the no of clients should be defined and hence a
> check for the same is done in MAF.
Defined where? What is "MAF"?
> Apart from all these the requests coming from all the clients is to be
> accumulated and based on that modem release or access should be
> performed, hence so.
That sentance makes no sense to me, it must be too early for me here...
greg k-h
> On Wed, Oct 03, 2012 at 05:54:08AM +0200, Arun MURTHY wrote:
> > > On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> > > > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/slab.h>
> > > > > > +#include <linux/err.h>
> > > > > > +#include <linux/printk.h>
> > > > > > +#include <linux/modem_shm/modem.h>
> > > > > > +
> > > > > > +static struct class *modem_class;
> > > > >
> > > > > What's wrong with a bus_type instead?
> > > >
> > > > Can I know the advantage of using bus_type over class?
> > >
> > > You have devices living on a bus, and it's much more descriptive
> > > than a class (which we are going to eventually get rid of one of these
> days...).
> > >
> > > Might I ask why you choose a class over a bus_type?
> >
> > Basically my requirement is to create a central entity for accessing
> > and releasing modem from APE.
>
> What is an "APE"?
>
> And what do you mean by "accessing" and "releasing"?
APE - Application Processor Engine
There are two processors but on a single chip, one being the APE and other
is the modem. So 'accessing' means requesting access or waking-up the
co-processor and releasing means allowing the co-processor to sleep.
>
> > Since this is done by different clients the central entity should be
> > able to handle the request and play safely, since this has more affect
> > in system suspend and deep sleep. Using class helps me in achieving
> > this and also create an entry to user space which can be used in the
> > later parts. Moreover this not something like a bus or so, so I didn't
> > use bus instead went with a simple class approach.
>
> But as you have devices that are "binding" to this "controller", a bus might
> make more sense, right?
Have explained above regarding the platform, the concept of bus doesn't
come into picture at all. Here its just waking-up the modem and allowing
it to go to sleep.
>
> I don't see how a class helps out for you here more than anything else, what
> are you expecting from the class interface? You aren't using the reference
> counting logic it provides, so why use it at all?
I am using the reference counting logic in class such as class_for_each_device.
>
> Actually, why use the driver core at all in the first place if you aren't needing
> the devices to show up in sysfs (as you don't have a device, you are just a
> mediator)?
Yes I am something like a mediator, but since this is associated with many
clients, there should be some central entity to take inputs from all the clients
and act accordingly. This MAF does that. Sysfs will also be created for this
MAF in the coming versions.
>
> > > > > > +int modem_release(struct modem_desc *mdesc) {
> > > > > > + if (!mdesc->release)
> > > > > > + return -EFAULT;
> > > > > > +
> > > > > > + if (modem_is_requested(mdesc)) {
> > > > > > + atomic_dec(&mdesc->mclients->cnt);
> > > > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> > > > > > + mdesc->release(mdesc);
> > > > > > + atomic_dec(&mdesc->use_cnt);
> > > > > > + }
> > > > >
> > > > > Eeek, why aren't you using the built-in reference counting that
> > > > > the struct device provided to you, and instead are rolling your own?
> > > > > This happens in many places, why?
> > > >
> > > > My usage of counters over here is for each modem there are many
> clients.
> > > > Each of the clients will have a ref to modem_desc. Each of them
> > > > use this for requesting and releasing the modem. One counter for
> > > > tracking the request and release for each client which is done by
> > > > variable 'cnt' in
> > > struct clients.
> > > > The counter use_cnt is used for tracking the modem request/release
> > > > irrespective of the clients and counter cli_cnt is used for
> > > > restricting the modem_get to the no of clients defined in no_clients.
> > > >
> > > > So totally 3 counter one for restricting the usage of modem_get by
> > > > clients, second for restricting modem request/release at top
> > > > level, and 3rd for restricting modem release/request for per
> > > > client per modem
> > > basis.
> > > >
> > > > Can you let me know if the same can be achieved by using built-in
> > > > ref counting?
> > >
> > > Yes, because you don't need all of those different levels, just
> > > stick with one and you should be fine. :)
> > >
> >
> > No, checks at all these levels are required, I have briefed out the need also.
>
> I still don't understand, sorry.
The pictorial view by Anish should help in understanding.
Modem Client1 Client2 Client3 Client4
State turn-on request
State no-state-change request
State no-state-change request
State no-state-change request
State no-state-change release
State no-state-change release
State no-state-change release
State turn-off release
This is just a simple straight forward example.
>
> > This will have effect on system power management, i.e suspend and deep
> > sleep.
>
> How does power management matter? If you tie into the driver model
> properly, power management comes "for free" so you don't have to do
> anything special about it. Why not use that logic instead of trying to roll your
> own?
As said there are two processors on a single chip playing over here. One being
the APE(Application Processor Engine) and other is Modem. Since they are on
a single chip but for APE entering into deep sleep modem should be released.
>
> > We restrict that the drivers should request modem only once and
> > release only once, but we cannot rely on the clients hence a check for
> > the same has to be done in the MAF.
>
> You can't rely on the clients to do what? And why can't you rely on them?
> What is going to happen? Who is a "client" here? Other kernel code?
Yes, let me take my driver itself as an example. Here the clients are the
shared memory driver, sim driver, security etc. Shared memory driver
is the communicating media between the APE and Modem and hence
needs to wake-up the modem and after completion should allow modem
to enter sleep.
Similarly it's the same for sim driver also.
We define that the clients such as shared memory driver and the sim
driver should request for modem only one and then release only once
and also since this is a MAF shouldn't it take care of checking the same?
>
> I really don't understand your model at all as to what you are trying to
> mediate and manage here, sorry. I suggest writing it all up as your first patch
> (documentation is good), so that we can properly review your
> implementation and not argue about how to implement something that I
> honestly don't understand.
Sorry for that. Actually my 4th patch in this patchset includes the documentation.
Since it's the kernel doc I have made it as the last patch in this patchset, else
kernel doc compilation will fail.
Please feel free to refer the 4th patch for the documentation part and if still
not clear I can provide more explanation on this.
>
> > Also the no of clients should be defined and hence a check for the
> > same is done in MAF.
>
> Defined where? What is "MAF"?
This driver is MAF(Modem Access Framework).
>
> > Apart from all these the requests coming from all the clients is to be
> > accumulated and based on that modem release or access should be
> > performed, hence so.
>
> That sentance makes no sense to me, it must be too early for me here...
Thanks and Regards,
Arun R Murthy
-----------------