[PATCH v2] sound: pci: ctxfi: pr_* replaced with dev

At Mon, 15 Sep 2014 19:39:41 +0530,

Post by Sudip Mukherjee
pr_* macros replaced with dev_* as they are more preffered over pr_*.
---
in v1 Takashi mentioned that now we have card->dev
so v2 is using card->dev as much as possible.
sound/pci/ctxfi/ctatc.c | 24 ++++++++++++++----------
sound/pci/ctxfi/cthw20k1.c | 15 +++++++++------
sound/pci/ctxfi/cthw20k2.c | 22 ++++++++++++++--------
sound/pci/ctxfi/ctmixer.c | 6 ++++--
sound/pci/ctxfi/ctpcm.c | 9 ++++++---
sound/pci/ctxfi/ctresource.c | 20 +++++++++++++-------
sound/pci/ctxfi/xfi.c | 15 +++++++++------
7 files changed, 69 insertions(+), 42 deletions(-)
diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c
index d92a08c..a786bc1 100644
--- a/sound/pci/ctxfi/ctatc.c
+++ b/sound/pci/ctxfi/ctatc.c
@@ -1282,8 +1282,8 @@ static int atc_identify_card(struct ct_atc *atc, unsigned int ssid)
p = snd_pci_quirk_lookup_id(vendor_id, device_id, list);
if (p) {
if (p->value < 0) {
- pr_err("ctxfi: Device %04x:%04x is black-listed\n",
- vendor_id, device_id);
+ dev_err(atc->card->dev, "ctxfi: Device %04x:%04x is black-listed\n",
+ vendor_id, device_id);

You need no prefix for dev_xxx().

Post by Sudip Mukherjee
diff --git a/sound/pci/ctxfi/ctresource.c b/sound/pci/ctxfi/ctresource.c
index e49d2be..80beecb 100644
--- a/sound/pci/ctxfi/ctresource.c
+++ b/sound/pci/ctxfi/ctresource.c

(snip)

Post by Sudip Mukherjee
@@ -282,8 +287,9 @@ int rsc_mgr_uninit(struct rsc_mgr *mgr)
break;
- pr_err("ctxfi: Invalid resource type value %d!\n",
- mgr->type);
+ dev_err(&(((struct hw *)mgr->hw)->pci->dev),
+ "ctxfi: Invalid resource type value %d!\n",
+ mgr->type);

Did you really conclude that this is the best way?
Also, is it good to mix up the usages of both card->dev and &pci->dev?
Think again.

thanks,

Takashi

Sudip Mukherjee

2014-09-15 15:25:54 UTC

Post by Takashi Iwai
At Mon, 15 Sep 2014 19:39:41 +0530,

You need no prefix for dev_xxx().

i was also thinking that . :)

Post by Sudip Mukherjee
diff --git a/sound/pci/ctxfi/ctresource.c b/sound/pci/ctxfi/ctresource.c
index e49d2be..80beecb 100644
--- a/sound/pci/ctxfi/ctresource.c
+++ b/sound/pci/ctxfi/ctresource.c

(snip)

Did you really conclude that this is the best way?
Also, is it good to mix up the usages of both card->dev and &pci->dev?
Think again.

i have a doubt regarding this :-
in the snd_card_new() card->dev is being assigned with &pci->dev ,
then are not they the same ?

i was trying to get some way of finding out the reference of card->dev from the resource manager ,
but ... :(
i will try again and if i cant find any way i will ask for some hint from you.

i am still a newbie , and started this pr_* to dev_* on your inspiration , and i hope
you will not be irritated by my patches . :)

thanks
sudip

Post by Takashi Iwai
thanks,
Takashi

Takashi Iwai

2014-09-15 18:17:52 UTC

At Mon, 15 Sep 2014 20:55:54 +0530,

Post by Takashi Iwai
At Mon, 15 Sep 2014 19:39:41 +0530,

You need no prefix for dev_xxx().

i was also thinking that . :)

Post by Sudip Mukherjee
diff --git a/sound/pci/ctxfi/ctresource.c b/sound/pci/ctxfi/ctresource.c
index e49d2be..80beecb 100644
--- a/sound/pci/ctxfi/ctresource.c
+++ b/sound/pci/ctxfi/ctresource.c

(snip)

Did you really conclude that this is the best way?
Also, is it good to mix up the usages of both card->dev and &pci->dev?
Think again.

i have a doubt regarding this :-
in the snd_card_new() card->dev is being assigned with &pci->dev ,
then are not they the same ?

Yes, but how can it be guaranteed in future? We may avoid the
problems in future by keeping the consistency at this moment. It's
one of the good points of keeping code consistent, in addition to:
increased readability / understandability and making the bug easier to
be spotted.

Post by Sudip Mukherjee
i was trying to get some way of finding out the reference of card->dev from the resource manager ,
but ... :(
i will try again and if i cant find any way i will ask for some hint from you.

Good! A hint is that there is no 100% perfect way to achieve this.
It's always compromise, and you'll have to choose which one is better
than others. For that, you'll have to evaluate multiple
implementations, and it's a really good exercise for coding.

Post by Sudip Mukherjee
i am still a newbie , and started this pr_* to dev_* on your inspiration , and i hope
you will not be irritated by my patches . :)

No problem. I left these driver codes just because of laziness, as I
knew to work more on the code than the simple scripting :)

Takashi

Sudip Mukherjee

2014-09-18 09:21:26 UTC

Post by Takashi Iwai
At Mon, 15 Sep 2014 20:55:54 +0530,

Post by Takashi Iwai
At Mon, 15 Sep 2014 19:39:41 +0530,

<snip>

Post by Sudip Mukherjee
diff --git a/sound/pci/ctxfi/ctresource.c b/sound/pci/ctxfi/ctresource.c
index e49d2be..80beecb 100644
--- a/sound/pci/ctxfi/ctresource.c
+++ b/sound/pci/ctxfi/ctresource.c

(snip)

Did you really conclude that this is the best way?
Also, is it good to mix up the usages of both card->dev and &pci->dev?
Think again.

i have a doubt regarding this :-
in the snd_card_new() card->dev is being assigned with &pci->dev ,
then are not they the same ?

Yes, but how can it be guaranteed in future? We may avoid the
problems in future by keeping the consistency at this moment. It's
increased readability / understandability and making the bug easier to
be spotted.

understood the point.

i can find the rsc_mgr from the snd_card or ct_atc
but if i want to find the reference to the device from the managers , then
either i can go with
1) &(((struct hw *)mgr->hw)->pci->dev)
or
2) use pci_get_drvdata with ((struct hw *)mgr->hw)->pci
or
3) using container_of with &(((struct hw *)mgr->hw)->pci->dev) to find the pointer to snd_card via card_dev , but it becomes too complicated :(

we can get to all the managers (rsc_mgr, amixer_mgr, src_mgr ...) using atc->rsc_mgrs[] , then there should be some simple way to go
the opposite way (reaching atc from the managers) . container_of will not work .. :(
or am i missing something??

thanks
sudip

Post by Sudip Mukherjee
i am still a newbie , and started this pr_* to dev_* on your inspiration , and i hope
you will not be irritated by my patches . :)

No problem. I left these driver codes just because of laziness, as I
knew to work more on the code than the simple scripting :)
Takashi

Takashi Iwai

2014-09-18 10:21:20 UTC

At Thu, 18 Sep 2014 14:51:26 +0530,

Post by Takashi Iwai
At Mon, 15 Sep 2014 20:55:54 +0530,

Post by Takashi Iwai
At Mon, 15 Sep 2014 19:39:41 +0530,

<snip>

Post by Sudip Mukherjee
diff --git a/sound/pci/ctxfi/ctresource.c b/sound/pci/ctxfi/ctresource.c
index e49d2be..80beecb 100644
--- a/sound/pci/ctxfi/ctresource.c
+++ b/sound/pci/ctxfi/ctresource.c

(snip)

Did you really conclude that this is the best way?
Also, is it good to mix up the usages of both card->dev and &pci->dev?
Think again.

i have a doubt regarding this :-
in the snd_card_new() card->dev is being assigned with &pci->dev ,
then are not they the same ?

understood the point.

i can find the rsc_mgr from the snd_card or ct_atc
but if i want to find the reference to the device from the managers , then
either i can go with
1) &(((struct hw *)mgr->hw)->pci->dev)
or
2) use pci_get_drvdata with ((struct hw *)mgr->hw)->pci
or
3) using container_of with &(((struct hw *)mgr->hw)->pci->dev) to find the pointer to snd_card via card_dev , but it becomes too complicated :(
we can get to all the managers (rsc_mgr, amixer_mgr, src_mgr ...) using atc->rsc_mgrs[] , then there should be some simple way to go
the opposite way (reaching atc from the managers) . container_of will not work .. :(
or am i missing something??

Adding a new field pointing to card to each struct.

Takashi

Sudip Mukherjee

2014-09-18 11:31:25 UTC

Post by Takashi Iwai
At Thu, 18 Sep 2014 14:51:26 +0530,

Post by Takashi Iwai
At Mon, 15 Sep 2014 20:55:54 +0530,

Post by Takashi Iwai
At Mon, 15 Sep 2014 19:39:41 +0530,

<snip>

Post by Sudip Mukherjee
diff --git a/sound/pci/ctxfi/ctresource.c b/sound/pci/ctxfi/ctresource.c
index e49d2be..80beecb 100644
--- a/sound/pci/ctxfi/ctresource.c
+++ b/sound/pci/ctxfi/ctresource.c

(snip)

Did you really conclude that this is the best way?
Also, is it good to mix up the usages of both card->dev and &pci->dev?
Think again.

i have a doubt regarding this :-
in the snd_card_new() card->dev is being assigned with &pci->dev ,
then are not they the same ?

understood the point.

i can find the rsc_mgr from the snd_card or ct_atc
but if i want to find the reference to the device from the managers , then
either i can go with
1) &(((struct hw *)mgr->hw)->pci->dev)
or
2) use pci_get_drvdata with ((struct hw *)mgr->hw)->pci
or
3) using container_of with &(((struct hw *)mgr->hw)->pci->dev) to find the pointer to snd_card via card_dev , but it becomes too complicated :(
we can get to all the managers (rsc_mgr, amixer_mgr, src_mgr ...) using atc->rsc_mgrs[] , then there should be some simple way to go
the opposite way (reaching atc from the managers) . container_of will not work .. :(
or am i missing something??

Adding a new field pointing to card to each struct.

i seriously did not think of that , i was under the impression that a newbie is not supposed to change or add to the designs.
anyways, i feel that i understand the code a little better than what i understood at the the time of my first patch.
and definitely your "writing-an-alsa-driver" has helped a lot .

thanks
sudip

Post by Takashi Iwai
Takashi

Joe Perches

2014-09-15 14:35:01 UTC