| Message ID | 1411315421-24138-1-git-send-email-sudipm.mukherjee@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Takashi Iwai |
| Headers | show |
At Sun, 21 Sep 2014 21:33:37 +0530, Sudip Mukherjee wrote: > > added and initialized a pointer of snd_card in the struct hw to get a reference of > the card in other functions You need to explain why this change is needed. The above text shows what it'd do, but it doesn't give any explanation to convince why this patch must be merged in. In this case, it's a preliminary patch for the changes to convert to dev_*() later. It's the most important bit, so don't forget it. thanks, Takashi > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > --- > sound/pci/ctxfi/ctatc.c | 1 + > sound/pci/ctxfi/cthardware.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c > index d92a08c..b21eda4 100644 > --- a/sound/pci/ctxfi/ctatc.c > +++ b/sound/pci/ctxfi/ctatc.c > @@ -1333,6 +1333,7 @@ static int atc_create_hw_devs(struct ct_atc *atc) > pr_err("Failed to create hw obj!!!\n"); > return err; > } > + hw->card = atc->card; > atc->hw = hw; > > /* Initialize card hardware. */ > diff --git a/sound/pci/ctxfi/cthardware.h b/sound/pci/ctxfi/cthardware.h > index 5977e9a..940bbd0 100644 > --- a/sound/pci/ctxfi/cthardware.h > +++ b/sound/pci/ctxfi/cthardware.h > @@ -20,6 +20,7 @@ > > #include <linux/types.h> > #include <linux/pci.h> > +#include <sound/core.h> > > enum CHIPTYP { > ATC20K1, > @@ -184,6 +185,7 @@ struct hw { > void *irq_callback_data; > > struct pci_dev *pci; /* the pci kernel structure of this card */ > + struct snd_card *card; /* pointer to this card */ > int irq; > unsigned long io_base; > unsigned long mem_base; > -- > 1.8.1.2 >
On Mon, Sep 22, 2014 at 09:04:53AM +0200, Takashi Iwai wrote: > At Sun, 21 Sep 2014 21:33:37 +0530, > Sudip Mukherjee wrote: > > > > added and initialized a pointer of snd_card in the struct hw to get a reference of > > the card in other functions > > You need to explain why this change is needed. The above text shows > what it'd do, but it doesn't give any explanation to convince why this > patch must be merged in. In this case, it's a preliminary patch for > the changes to convert to dev_*() later. It's the most important bit, > so don't forget it. > then do i mention in the commit that it is the preliminary patch for the later changes to dev_* ? i was thinking that we can get a reference of the card from other functions which are using struct hw is a reason by itself. thanks sudip > > thanks, > > Takashi > > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > > --- > > sound/pci/ctxfi/ctatc.c | 1 + > > sound/pci/ctxfi/cthardware.h | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c > > index d92a08c..b21eda4 100644 > > --- a/sound/pci/ctxfi/ctatc.c > > +++ b/sound/pci/ctxfi/ctatc.c > > @@ -1333,6 +1333,7 @@ static int atc_create_hw_devs(struct ct_atc *atc) > > pr_err("Failed to create hw obj!!!\n"); > > return err; > > } > > + hw->card = atc->card; > > atc->hw = hw; > > > > /* Initialize card hardware. */ > > diff --git a/sound/pci/ctxfi/cthardware.h b/sound/pci/ctxfi/cthardware.h > > index 5977e9a..940bbd0 100644 > > --- a/sound/pci/ctxfi/cthardware.h > > +++ b/sound/pci/ctxfi/cthardware.h > > @@ -20,6 +20,7 @@ > > > > #include <linux/types.h> > > #include <linux/pci.h> > > +#include <sound/core.h> > > > > enum CHIPTYP { > > ATC20K1, > > @@ -184,6 +185,7 @@ struct hw { > > void *irq_callback_data; > > > > struct pci_dev *pci; /* the pci kernel structure of this card */ > > + struct snd_card *card; /* pointer to this card */ > > int irq; > > unsigned long io_base; > > unsigned long mem_base; > > -- > > 1.8.1.2 > >
At Mon, 22 Sep 2014 13:45:41 +0530, Sudip Mukherjee wrote: > > On Mon, Sep 22, 2014 at 09:04:53AM +0200, Takashi Iwai wrote: > > At Sun, 21 Sep 2014 21:33:37 +0530, > > Sudip Mukherjee wrote: > > > > > > added and initialized a pointer of snd_card in the struct hw to get a reference of > > > the card in other functions > > > > You need to explain why this change is needed. The above text shows > > what it'd do, but it doesn't give any explanation to convince why this > > patch must be merged in. In this case, it's a preliminary patch for > > the changes to convert to dev_*() later. It's the most important bit, > > so don't forget it. > > > then do i mention in the commit that it is the preliminary patch for the later changes to dev_* ? If the purpose of this patch is so, just write what it'll be used for. > i was thinking that we can get a reference of the card from other functions which are using struct hw > is a reason by itself. If the purpose of this patch is rather broader, just write what it'll be used for. Do you see what was missing? Takashi
On Mon, Sep 22, 2014 at 10:21:19AM +0200, Takashi Iwai wrote: > At Mon, 22 Sep 2014 13:45:41 +0530, > Sudip Mukherjee wrote: > > > > On Mon, Sep 22, 2014 at 09:04:53AM +0200, Takashi Iwai wrote: > > > At Sun, 21 Sep 2014 21:33:37 +0530, > > > Sudip Mukherjee wrote: > > > > > > > > added and initialized a pointer of snd_card in the struct hw to get a reference of > > > > the card in other functions > > > > > > You need to explain why this change is needed. The above text shows > > > what it'd do, but it doesn't give any explanation to convince why this > > > patch must be merged in. In this case, it's a preliminary patch for > > > the changes to convert to dev_*() later. It's the most important bit, > > > so don't forget it. > > > > > then do i mention in the commit that it is the preliminary patch for the later changes to dev_* ? > > If the purpose of this patch is so, just write what it'll be used > for. > > > i was thinking that we can get a reference of the card from other functions which are using struct hw > > is a reason by itself. > > If the purpose of this patch is rather broader, just write what it'll > be used for. > > Do you see what was missing? > understood. thanks . I hope v4 will be a better series than this . sudip > > Takashi
diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c index d92a08c..b21eda4 100644 --- a/sound/pci/ctxfi/ctatc.c +++ b/sound/pci/ctxfi/ctatc.c @@ -1333,6 +1333,7 @@ static int atc_create_hw_devs(struct ct_atc *atc) pr_err("Failed to create hw obj!!!\n"); return err; } + hw->card = atc->card; atc->hw = hw; /* Initialize card hardware. */ diff --git a/sound/pci/ctxfi/cthardware.h b/sound/pci/ctxfi/cthardware.h index 5977e9a..940bbd0 100644 --- a/sound/pci/ctxfi/cthardware.h +++ b/sound/pci/ctxfi/cthardware.h @@ -20,6 +20,7 @@ #include <linux/types.h> #include <linux/pci.h> +#include <sound/core.h> enum CHIPTYP { ATC20K1, @@ -184,6 +185,7 @@ struct hw { void *irq_callback_data; struct pci_dev *pci; /* the pci kernel structure of this card */ + struct snd_card *card; /* pointer to this card */ int irq; unsigned long io_base; unsigned long mem_base;
added and initialized a pointer of snd_card in the struct hw to get a reference of the card in other functions Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> --- sound/pci/ctxfi/ctatc.c | 1 + sound/pci/ctxfi/cthardware.h | 2 ++ 2 files changed, 3 insertions(+)