All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cdmamodem: create vendor.h file.
@ 2011-07-23 11:04 Bertrand Aygon
  2011-07-23 11:08 ` Aygon, Bertrand
  2011-07-23 15:05 ` Marcel Holtmann
  0 siblings, 2 replies; 8+ messages in thread
From: Bertrand Aygon @ 2011-07-23 11:04 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2330 bytes --]

---
 Makefile.am                |    3 ++-
 drivers/cdmamodem/vendor.h |   25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletions(-)
 create mode 100644 drivers/cdmamodem/vendor.h

diff --git a/Makefile.am b/Makefile.am
index e00f73b..f1ef96c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -270,7 +270,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
 			drivers/cdmamodem/voicecall.c \
 			drivers/cdmamodem/devinfo.c \
 			drivers/cdmamodem/connman.c \
-			drivers/cdmamodem/network-registration.c
+			drivers/cdmamodem/network-registration.c \
+			drivers/cdmamodem/vendor.h
 endif
 
 builtin_modules += g1
diff --git a/drivers/cdmamodem/vendor.h b/drivers/cdmamodem/vendor.h
new file mode 100644
index 0000000..3b00bbf
--- /dev/null
+++ b/drivers/cdmamodem/vendor.h
@@ -0,0 +1,25 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+enum ofono_vendor {
+	OFONO_VENDOR_GENERIC = 0,
+	OFONO_VENDOR_SPEEDUP,
+};
-- 
1.7.4.1

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH] cdmamodem: create vendor.h file.
  2011-07-23 11:04 [PATCH] cdmamodem: create vendor.h file Bertrand Aygon
@ 2011-07-23 11:08 ` Aygon, Bertrand
  2011-07-23 15:05 ` Marcel Holtmann
  1 sibling, 0 replies; 8+ messages in thread
From: Aygon, Bertrand @ 2011-07-23 11:08 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 3575 bytes --]

Hi,

Sorry I have just see that this patch require my previous patch on CDMA Network Registration.

Please ignore it for now.

Regards,

Bertrand

> -----Original Message-----
> From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf Of
> Bertrand Aygon
> Sent: Saturday, July 23, 2011 1:04 PM
> To: ofono(a)ofono.org
> Subject: [PATCH] cdmamodem: create vendor.h file.
> 
> ---
>  Makefile.am                |    3 ++-
>  drivers/cdmamodem/vendor.h |   25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/cdmamodem/vendor.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index e00f73b..f1ef96c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -270,7 +270,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
>  			drivers/cdmamodem/voicecall.c \
>  			drivers/cdmamodem/devinfo.c \
>  			drivers/cdmamodem/connman.c \
> -			drivers/cdmamodem/network-registration.c
> +			drivers/cdmamodem/network-registration.c \
> +			drivers/cdmamodem/vendor.h
>  endif
> 
>  builtin_modules += g1
> diff --git a/drivers/cdmamodem/vendor.h b/drivers/cdmamodem/vendor.h
> new file mode 100644
> index 0000000..3b00bbf
> --- /dev/null
> +++ b/drivers/cdmamodem/vendor.h
> @@ -0,0 +1,25 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301
> USA
> + *
> + */
> +
> +enum ofono_vendor {
> +	OFONO_VENDOR_GENERIC = 0,
> +	OFONO_VENDOR_SPEEDUP,
> +};
> --
> 1.7.4.1
> 
> ---------------------------------------------------------------------
> Intel Corporation SAS (French simplified joint stock company)
> Registered headquarters: "Les Montalets"- 2, rue de Paris,
> 92196 Meudon Cedex, France
> Registration Number:  302 456 199 R.C.S. NANTERRE
> Capital: 4,572,000 Euros
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] cdmamodem: create vendor.h file.
  2011-07-23 11:04 [PATCH] cdmamodem: create vendor.h file Bertrand Aygon
  2011-07-23 11:08 ` Aygon, Bertrand
@ 2011-07-23 15:05 ` Marcel Holtmann
  2011-07-25 14:30   ` Aygon, Bertrand
  1 sibling, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2011-07-23 15:05 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]

Hi Bertrand,

>  Makefile.am                |    3 ++-
>  drivers/cdmamodem/vendor.h |   25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/cdmamodem/vendor.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index e00f73b..f1ef96c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -270,7 +270,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
>  			drivers/cdmamodem/voicecall.c \
>  			drivers/cdmamodem/devinfo.c \
>  			drivers/cdmamodem/connman.c \
> -			drivers/cdmamodem/network-registration.c
> +			drivers/cdmamodem/network-registration.c \
> +			drivers/cdmamodem/vendor.h
>  endif
>  
>  builtin_modules += g1
> diff --git a/drivers/cdmamodem/vendor.h b/drivers/cdmamodem/vendor.h
> new file mode 100644
> index 0000000..3b00bbf
> --- /dev/null
> +++ b/drivers/cdmamodem/vendor.h
> @@ -0,0 +1,25 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +enum ofono_vendor {
> +	OFONO_VENDOR_GENERIC = 0,
> +	OFONO_VENDOR_SPEEDUP,
> +};

do not bother with this until it is really needed. Vendor quirks are the
last resort to avoid massive code duplication. In a lot of cases a
vendor specific atom driver is the better choice.

And you can just share the atmodem ones in this case anyway.

Regards

Marcel



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] cdmamodem: create vendor.h file.
  2011-07-25 15:00       ` Aygon, Bertrand
@ 2011-07-25  6:23         ` Denis Kenzior
  2011-07-25 16:58         ` Marcel Holtmann
  1 sibling, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2011-07-25  6:23 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 4391 bytes --]

Hi Bertrand,

On 07/25/2011 10:00 AM, Aygon, Bertrand wrote:
> Hi,
> 
>>>>>  Makefile.am                |    3 ++-
>>>>>  drivers/cdmamodem/vendor.h |   25 +++++++++++++++++++++++++
>>>>>  2 files changed, 27 insertions(+), 1 deletions(-)
>>>>>  create mode 100644 drivers/cdmamodem/vendor.h
>>>>>
>>>>> diff --git a/Makefile.am b/Makefile.am
>>>>> index e00f73b..f1ef96c 100644
>>>>> --- a/Makefile.am
>>>>> +++ b/Makefile.am
>>>>> @@ -270,7 +270,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
>>>>>  			drivers/cdmamodem/voicecall.c \
>>>>>  			drivers/cdmamodem/devinfo.c \
>>>>>  			drivers/cdmamodem/connman.c \
>>>>> -			drivers/cdmamodem/network-registration.c
>>>>> +			drivers/cdmamodem/network-registration.c \
>>>>> +			drivers/cdmamodem/vendor.h
>>>>>  endif
>>>>>
>>>>>  builtin_modules += g1
>>>>> diff --git a/drivers/cdmamodem/vendor.h b/drivers/cdmamodem/vendor.h
>>>>> new file mode 100644
>>>>> index 0000000..3b00bbf
>>>>> --- /dev/null
>>>>> +++ b/drivers/cdmamodem/vendor.h
>>>>> @@ -0,0 +1,25 @@
>>>>> +/*
>>>>> + *
>>>>> + *  oFono - Open Source Telephony
>>>>> + *
>>>>> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
>>>>> + *
>>>>> + *  This program is free software; you can redistribute it and/or
>> modify
>>>>> + *  it under the terms of the GNU General Public License version 2 as
>>>>> + *  published by the Free Software Foundation.
>>>>> + *
>>>>> + *  This program is distributed in the hope that it will be useful,
>>>>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> + *  GNU General Public License for more details.
>>>>> + *
>>>>> + *  You should have received a copy of the GNU General Public License
>>>>> + *  along with this program; if not, write to the Free Software
>>>>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-
>> 1301
>>>> USA
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +enum ofono_vendor {
>>>>> +	OFONO_VENDOR_GENERIC = 0,
>>>>> +	OFONO_VENDOR_SPEEDUP,
>>>>> +};
>>>>
>>>> do not bother with this until it is really needed. Vendor quirks are the
>>>> last resort to avoid massive code duplication. In a lot of cases a
>>>> vendor specific atom driver is the better choice.
>>>>
>>>> And you can just share the atmodem ones in this case anyway.
>>>
>>> I am thinking again to this file, and I think we will definitely need a
>> Vendor.h for CDMA.
>>>
>>> As we said in another discussion, there is no standard in CDMA, so we should
>> have vendor specific code in any atom. And creating an atom per modem will
>> duplicate a lot of code.
>>>
>>> And I don't know if we should set an OFONO_VENDOR_GENERIC for CDMA. I think
>> that if 2 modems are using the same AT command (CPIN for example), we should
>> do a switch or if/else with all the different vendor.
>>>
>>> Another exemple. We are working on ConnMan atom. The 2 kind of dongle we
>> have used, Huawei and SpeedUp, are using ^DORMANT for dormant mode
>> notification. I don't think that we have to duplicate the ConnMan atom. We
>> should be using the same atom, and use vendor define to set the AT command to
>> register.
>>>
>>> What do you think about this?
>>
>> are you sure that the SpeedUp and Huawei dongle are really different.
>> Maybe it is just a re-branded version of the other. They could be
>> different and just share the same AT command set, but even then we might
>> not just bother here.
> 
> I do not remember exactly, I will have to go through my log, but from what I remember, they are sharing lot of command.
> 

Please stop over-analyzing.  Write a huaweimodem driver for network
registration.  If it fits the bill for speedup, then just use it from
the speedup modem driver.

Based on the various CDMA docs I've read there won't be a generic netreg
atom implementation anyway, they're all different.

>> And from were I am standing, feel free to share the vendor quirks from
>> atmodem/vendor.h. At least until we have the rest figured out.
> 
> Ok, and what about the GENERIC, are we creating one? I don't think that this is a good idea.
> 

This doesn't make sense in the first place.  If something is generic
then it belongs in drivers/cdmamodem and no quirk is required.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] cdmamodem: create vendor.h file.
  2011-07-23 15:05 ` Marcel Holtmann
@ 2011-07-25 14:30   ` Aygon, Bertrand
  2011-07-25 14:44     ` Marcel Holtmann
  0 siblings, 1 reply; 8+ messages in thread
From: Aygon, Bertrand @ 2011-07-25 14:30 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 3642 bytes --]

Hi Marcel,

> >  Makefile.am                |    3 ++-
> >  drivers/cdmamodem/vendor.h |   25 +++++++++++++++++++++++++
> >  2 files changed, 27 insertions(+), 1 deletions(-)
> >  create mode 100644 drivers/cdmamodem/vendor.h
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index e00f73b..f1ef96c 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -270,7 +270,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
> >  			drivers/cdmamodem/voicecall.c \
> >  			drivers/cdmamodem/devinfo.c \
> >  			drivers/cdmamodem/connman.c \
> > -			drivers/cdmamodem/network-registration.c
> > +			drivers/cdmamodem/network-registration.c \
> > +			drivers/cdmamodem/vendor.h
> >  endif
> >
> >  builtin_modules += g1
> > diff --git a/drivers/cdmamodem/vendor.h b/drivers/cdmamodem/vendor.h
> > new file mode 100644
> > index 0000000..3b00bbf
> > --- /dev/null
> > +++ b/drivers/cdmamodem/vendor.h
> > @@ -0,0 +1,25 @@
> > +/*
> > + *
> > + *  oFono - Open Source Telephony
> > + *
> > + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> > + *
> > + *  This program is free software; you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License version 2 as
> > + *  published by the Free Software Foundation.
> > + *
> > + *  This program is distributed in the hope that it will be useful,
> > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *  GNU General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with this program; if not, write to the Free Software
> > + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301
> USA
> > + *
> > + */
> > +
> > +enum ofono_vendor {
> > +	OFONO_VENDOR_GENERIC = 0,
> > +	OFONO_VENDOR_SPEEDUP,
> > +};
> 
> do not bother with this until it is really needed. Vendor quirks are the
> last resort to avoid massive code duplication. In a lot of cases a
> vendor specific atom driver is the better choice.
> 
> And you can just share the atmodem ones in this case anyway.

I am thinking again to this file, and I think we will definitely need a Vendor.h for CDMA.

As we said in another discussion, there is no standard in CDMA, so we should have vendor specific code in any atom. And creating an atom per modem will duplicate a lot of code.

And I don't know if we should set an OFONO_VENDOR_GENERIC for CDMA. I think that if 2 modems are using the same AT command (CPIN for example), we should do a switch or if/else with all the different vendor.

Another exemple. We are working on ConnMan atom. The 2 kind of dongle we have used, Huawei and SpeedUp, are using ^DORMANT for dormant mode notification. I don't think that we have to duplicate the ConnMan atom. We should be using the same atom, and use vendor define to set the AT command to register.

What do you think about this?

Regards,

Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] cdmamodem: create vendor.h file.
  2011-07-25 14:30   ` Aygon, Bertrand
@ 2011-07-25 14:44     ` Marcel Holtmann
  2011-07-25 15:00       ` Aygon, Bertrand
  0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2011-07-25 14:44 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 3584 bytes --]

Hi Bertrand,

> > >  Makefile.am                |    3 ++-
> > >  drivers/cdmamodem/vendor.h |   25 +++++++++++++++++++++++++
> > >  2 files changed, 27 insertions(+), 1 deletions(-)
> > >  create mode 100644 drivers/cdmamodem/vendor.h
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index e00f73b..f1ef96c 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -270,7 +270,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
> > >  			drivers/cdmamodem/voicecall.c \
> > >  			drivers/cdmamodem/devinfo.c \
> > >  			drivers/cdmamodem/connman.c \
> > > -			drivers/cdmamodem/network-registration.c
> > > +			drivers/cdmamodem/network-registration.c \
> > > +			drivers/cdmamodem/vendor.h
> > >  endif
> > >
> > >  builtin_modules += g1
> > > diff --git a/drivers/cdmamodem/vendor.h b/drivers/cdmamodem/vendor.h
> > > new file mode 100644
> > > index 0000000..3b00bbf
> > > --- /dev/null
> > > +++ b/drivers/cdmamodem/vendor.h
> > > @@ -0,0 +1,25 @@
> > > +/*
> > > + *
> > > + *  oFono - Open Source Telephony
> > > + *
> > > + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> > > + *
> > > + *  This program is free software; you can redistribute it and/or modify
> > > + *  it under the terms of the GNU General Public License version 2 as
> > > + *  published by the Free Software Foundation.
> > > + *
> > > + *  This program is distributed in the hope that it will be useful,
> > > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + *  GNU General Public License for more details.
> > > + *
> > > + *  You should have received a copy of the GNU General Public License
> > > + *  along with this program; if not, write to the Free Software
> > > + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301
> > USA
> > > + *
> > > + */
> > > +
> > > +enum ofono_vendor {
> > > +	OFONO_VENDOR_GENERIC = 0,
> > > +	OFONO_VENDOR_SPEEDUP,
> > > +};
> > 
> > do not bother with this until it is really needed. Vendor quirks are the
> > last resort to avoid massive code duplication. In a lot of cases a
> > vendor specific atom driver is the better choice.
> > 
> > And you can just share the atmodem ones in this case anyway.
> 
> I am thinking again to this file, and I think we will definitely need a Vendor.h for CDMA.
> 
> As we said in another discussion, there is no standard in CDMA, so we should have vendor specific code in any atom. And creating an atom per modem will duplicate a lot of code.
> 
> And I don't know if we should set an OFONO_VENDOR_GENERIC for CDMA. I think that if 2 modems are using the same AT command (CPIN for example), we should do a switch or if/else with all the different vendor.
> 
> Another exemple. We are working on ConnMan atom. The 2 kind of dongle we have used, Huawei and SpeedUp, are using ^DORMANT for dormant mode notification. I don't think that we have to duplicate the ConnMan atom. We should be using the same atom, and use vendor define to set the AT command to register.
> 
> What do you think about this?

are you sure that the SpeedUp and Huawei dongle are really different.
Maybe it is just a re-branded version of the other. They could be
different and just share the same AT command set, but even then we might
not just bother here.

And from were I am standing, feel free to share the vendor quirks from
atmodem/vendor.h. At least until we have the rest figured out.

Regards

Marcel



^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] cdmamodem: create vendor.h file.
  2011-07-25 14:44     ` Marcel Holtmann
@ 2011-07-25 15:00       ` Aygon, Bertrand
  2011-07-25  6:23         ` Denis Kenzior
  2011-07-25 16:58         ` Marcel Holtmann
  0 siblings, 2 replies; 8+ messages in thread
From: Aygon, Bertrand @ 2011-07-25 15:00 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 4538 bytes --]

Hi,

> > > >  Makefile.am                |    3 ++-
> > > >  drivers/cdmamodem/vendor.h |   25 +++++++++++++++++++++++++
> > > >  2 files changed, 27 insertions(+), 1 deletions(-)
> > > >  create mode 100644 drivers/cdmamodem/vendor.h
> > > >
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index e00f73b..f1ef96c 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -270,7 +270,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
> > > >  			drivers/cdmamodem/voicecall.c \
> > > >  			drivers/cdmamodem/devinfo.c \
> > > >  			drivers/cdmamodem/connman.c \
> > > > -			drivers/cdmamodem/network-registration.c
> > > > +			drivers/cdmamodem/network-registration.c \
> > > > +			drivers/cdmamodem/vendor.h
> > > >  endif
> > > >
> > > >  builtin_modules += g1
> > > > diff --git a/drivers/cdmamodem/vendor.h b/drivers/cdmamodem/vendor.h
> > > > new file mode 100644
> > > > index 0000000..3b00bbf
> > > > --- /dev/null
> > > > +++ b/drivers/cdmamodem/vendor.h
> > > > @@ -0,0 +1,25 @@
> > > > +/*
> > > > + *
> > > > + *  oFono - Open Source Telephony
> > > > + *
> > > > + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> > > > + *
> > > > + *  This program is free software; you can redistribute it and/or
> modify
> > > > + *  it under the terms of the GNU General Public License version 2 as
> > > > + *  published by the Free Software Foundation.
> > > > + *
> > > > + *  This program is distributed in the hope that it will be useful,
> > > > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > + *  GNU General Public License for more details.
> > > > + *
> > > > + *  You should have received a copy of the GNU General Public License
> > > > + *  along with this program; if not, write to the Free Software
> > > > + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-
> 1301
> > > USA
> > > > + *
> > > > + */
> > > > +
> > > > +enum ofono_vendor {
> > > > +	OFONO_VENDOR_GENERIC = 0,
> > > > +	OFONO_VENDOR_SPEEDUP,
> > > > +};
> > >
> > > do not bother with this until it is really needed. Vendor quirks are the
> > > last resort to avoid massive code duplication. In a lot of cases a
> > > vendor specific atom driver is the better choice.
> > >
> > > And you can just share the atmodem ones in this case anyway.
> >
> > I am thinking again to this file, and I think we will definitely need a
> Vendor.h for CDMA.
> >
> > As we said in another discussion, there is no standard in CDMA, so we should
> have vendor specific code in any atom. And creating an atom per modem will
> duplicate a lot of code.
> >
> > And I don't know if we should set an OFONO_VENDOR_GENERIC for CDMA. I think
> that if 2 modems are using the same AT command (CPIN for example), we should
> do a switch or if/else with all the different vendor.
> >
> > Another exemple. We are working on ConnMan atom. The 2 kind of dongle we
> have used, Huawei and SpeedUp, are using ^DORMANT for dormant mode
> notification. I don't think that we have to duplicate the ConnMan atom. We
> should be using the same atom, and use vendor define to set the AT command to
> register.
> >
> > What do you think about this?
> 
> are you sure that the SpeedUp and Huawei dongle are really different.
> Maybe it is just a re-branded version of the other. They could be
> different and just share the same AT command set, but even then we might
> not just bother here.

I do not remember exactly, I will have to go through my log, but from what I remember, they are sharing lot of command.

> And from were I am standing, feel free to share the vendor quirks from
> atmodem/vendor.h. At least until we have the rest figured out.

Ok, and what about the GENERIC, are we creating one? I don't think that this is a good idea.

Regards,

Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] cdmamodem: create vendor.h file.
  2011-07-25 15:00       ` Aygon, Bertrand
  2011-07-25  6:23         ` Denis Kenzior
@ 2011-07-25 16:58         ` Marcel Holtmann
  1 sibling, 0 replies; 8+ messages in thread
From: Marcel Holtmann @ 2011-07-25 16:58 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

H Bertrand,

> > And from were I am standing, feel free to share the vendor quirks from
> > atmodem/vendor.h. At least until we have the rest figured out.
> 
> Ok, and what about the GENERIC, are we creating one? I don't think that this is a good idea.

the atom driver by default should implement generic behavior. Aka vendor
set to 0. Only quirk behavior should be implemented via this. And that
only if it shares like 95% of the functionality otherwise. If it not
share that much, then just add another atom driver. So clearly no to the
GENERIC thing.

Regards

Marcel



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-07-25 16:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-23 11:04 [PATCH] cdmamodem: create vendor.h file Bertrand Aygon
2011-07-23 11:08 ` Aygon, Bertrand
2011-07-23 15:05 ` Marcel Holtmann
2011-07-25 14:30   ` Aygon, Bertrand
2011-07-25 14:44     ` Marcel Holtmann
2011-07-25 15:00       ` Aygon, Bertrand
2011-07-25  6:23         ` Denis Kenzior
2011-07-25 16:58         ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.