linux-kernel - Re: [PATCH v2] sound: pci: ctxfi: pr_* replaced with dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hbnqd8c0v.wl-tiwai@suse.de>
Date:	Thu, 18 Sep 2014 12:21:20 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
Cc:	Jaroslav Kysela <perex@...ex.cz>, alsa-devel@...a-project.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] sound: pci: ctxfi: pr_* replaced with dev_*

At Thu, 18 Sep 2014 14:51:26 +0530,
Sudip Mukherjee wrote:
> 
> On Mon, Sep 15, 2014 at 08:17:52PM +0200, Takashi Iwai wrote:
> > At Mon, 15 Sep 2014 20:55:54 +0530,
> > Sudip Mukherjee wrote:
> > > 
> > > On Mon, Sep 15, 2014 at 04:29:48PM +0200, Takashi Iwai wrote:
> > > > At Mon, 15 Sep 2014 19:39:41 +0530,
> > > > Sudip Mukherjee wrote:
> <snip>
> > > > > 
> > > > > 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)
> > > > > @@ -282,8 +287,9 @@ int rsc_mgr_uninit(struct rsc_mgr *mgr)
> > > > >  		case SUM:
> > > > >  			break;
> > > > >  		default:
> > > > > -			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.
> > > > 
> > > 
> > > 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.
> 
> understood the point.
> > 
> > > 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.
> > 
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/