Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Thu, Oct 11, 2012 at 11:37:20AM +0200, Arun MURTHY wrote: > > On Tue, Oct 09, 2012 at 07:37:02AM +0200, Arun MURTHY wrote: > > > Any further comments? > > > > I was waiting for you to address all of the previous ones with a new set of > > patches before burdening you with anything new :) > > There are not any changes in the code, this review was more like just > explaining our platform, protocol, few terminology and design approach. That all needs to go into the patches you send next time around :) And I thought I did have some actual code comments in there somewhere... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> On Tue, Oct 09, 2012 at 07:37:02AM +0200, Arun MURTHY wrote: > > Any further comments? > > I was waiting for you to address all of the previous ones with a new set of > patches before burdening you with anything new :) There are not any changes in the code, this review was more like just explaining our platform, protocol, few terminology and design approach. Thanks and Regards, Arun R Murthy -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Tue, Oct 09, 2012 at 07:37:02AM +0200, Arun MURTHY wrote: Any further comments? I was waiting for you to address all of the previous ones with a new set of patches before burdening you with anything new :) There are not any changes in the code, this review was more like just explaining our platform, protocol, few terminology and design approach. Thanks and Regards, Arun R Murthy -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Thu, Oct 11, 2012 at 11:37:20AM +0200, Arun MURTHY wrote: On Tue, Oct 09, 2012 at 07:37:02AM +0200, Arun MURTHY wrote: Any further comments? I was waiting for you to address all of the previous ones with a new set of patches before burdening you with anything new :) There are not any changes in the code, this review was more like just explaining our platform, protocol, few terminology and design approach. That all needs to go into the patches you send next time around :) And I thought I did have some actual code comments in there somewhere... greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Tue, Oct 09, 2012 at 07:37:02AM +0200, Arun MURTHY wrote: > Any further comments? I was waiting for you to address all of the previous ones with a new set of patches before burdening you with anything new :) greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Tue, Oct 09, 2012 at 07:37:02AM +0200, Arun MURTHY wrote: Any further comments? I was waiting for you to address all of the previous ones with a new set of patches before burdening you with anything new :) greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> > 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
> > > > > > > +#include
> > > > > > > +#include
> > > > > > > +#include
> > > > > > > +#include
> > > > > > > +
> > > > > > > +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(>mclients->cnt);
> > > > > > > + if (atomic_read(>use_cnt) == 1) {
> > > > > > > + mdesc->release(mdesc);
> > > > > > > + atomic_dec(>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 Client2Client3Client4
> State turn-on request
> State no-state-change request
> State no-state-change request
> State
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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 Client2Client3Client4
State turn-on request
State no-state-change request
State no-state-change request
State no-state-changerequest
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
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> 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
> > > > > > +#include
> > > > > > +#include
> > > > > > +#include
> > > > > > +#include
> > > > > > +
> > > > > > +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(>mclients->cnt);
> > > > > > + if (atomic_read(>use_cnt) == 1) {
> > > > > > + mdesc->release(mdesc);
> > > > > > + atomic_dec(>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 Client2Client3Client4
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
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +
> > > > > +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(>mclients->cnt);
> > > > > + if (atomic_read(>use_cnt) == 1) {
> > > > > + mdesc->release(mdesc);
> > > > > + atomic_dec(>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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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 Client2Client3Client4
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-offrelease
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
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> On Wed, Oct 3, 2012 at 9:24 AM, 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
> >> > > > +#include
> >> > > > +#include
> >> > > > +#include
> >> > > > +#include
> >> > > > +
> >> > > > +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(>mclients->cnt);
> >> > > > + if (atomic_read(>use_cnt) == 1) {
> >> > > > + mdesc->release(mdesc);
> >> > > > + atomic_dec(>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
--
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Wed, Oct 3, 2012 at 9:24 AM, 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
>> > > > +#include
>> > > > +#include
>> > > > +#include
>> > > > +#include
>> > > > +
>> > > > +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(>mclients->cnt);
>> > > > + if (atomic_read(>use_cnt) == 1) {
>> > > > + mdesc->release(mdesc);
>> > > > + atomic_dec(>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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> 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
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +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(>mclients->cnt);
> > > > + if (atomic_read(>use_cnt) == 1) {
> > > > + mdesc->release(mdesc);
> > > > + atomic_dec(>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
---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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
---
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Wed, Oct 3, 2012 at 9:24 AM, Arun MURTHY arun.mur...@stericsson.com 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Wed, Oct 3, 2012 at 9:24 AM, Arun MURTHY
arun.mur...@stericsson.com 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
--
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +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(>mclients->cnt);
> > > + if (atomic_read(>use_cnt) == 1) {
> > > + mdesc->release(mdesc);
> > > + atomic_dec(>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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> >> >> > +int modem_release(struct modem_desc *mdesc) {
> >> >> > + if (!mdesc->release)
> >> >> > + return -EFAULT;
> >> >> > +
> >> >> > + if (modem_is_requested(mdesc)) {
> >> >> > + atomic_dec(>mclients->cnt);
> >> >> > + if (atomic_read(>use_cnt) == 1) {
> >> >> > + mdesc->release(mdesc);
> >> >> > + atomic_dec(>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 Client2Client3Client4
> 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�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Mon, Oct 1, 2012 at 12:27 PM, Arun MURTHY wrote:
>> On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY
>> wrote:
>> >> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
>> >> > +#include
>> >> > +#include
>> >> > +#include
>> >> > +#include
>> >> > +#include
>> >> > +
>> >> > +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(>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(>mclients->cnt);
>> >> > + if (atomic_read(>use_cnt) == 1) {
>> >> > + mdesc->release(mdesc);
>> >> > + atomic_dec(>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 Client2Client3Client4
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
> --
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY
> wrote:
> >> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> >> > +#include
> >> > +#include
> >> > +#include
> >> > +#include
> >> > +#include
> >> > +
> >> > +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(>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(>mclients->cnt);
> >> > + if (atomic_read(>use_cnt) == 1) {
> >> > + mdesc->release(mdesc);
> >> > + atomic_dec(>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
--
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY wrote:
>> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +
>> > +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(>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(>mclients->cnt);
>> > + if (atomic_read(>use_cnt) == 1) {
>> > + mdesc->release(mdesc);
>> > + atomic_dec(>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(_dev->entry, _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(_dev_list_lock);
list_del(>entry);
mutex_unlock(_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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY arun.mur...@stericsson.com 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY
arun.mur...@stericsson.com 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
--
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Mon, Oct 1, 2012 at 12:27 PM, Arun MURTHY arun.mur...@stericsson.com wrote:
On Mon, Oct 1, 2012 at 11:00 AM, Arun MURTHY
arun.mur...@stericsson.com 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 Client2Client3Client4
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
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
+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 Client2Client3Client4
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�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
> On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +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(>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(>mclients->cnt);
> > + if (atomic_read(>use_cnt) == 1) {
> > + mdesc->release(mdesc);
> > + atomic_dec(>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
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static struct class *modem_class;
What's wrong with a bus_type instead?
> +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(>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?
> +
> +int modem_release(struct modem_desc *mdesc)
> +{
> + if (!mdesc->release)
> + return -EFAULT;
> +
> + if (modem_is_requested(mdesc)) {
> + atomic_dec(>mclients->cnt);
> + if (atomic_read(>use_cnt) == 1) {
> + mdesc->release(mdesc);
> + atomic_dec(>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?
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
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?
+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?
+
+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?
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
