| Message ID | 1455572189-11724-1-git-send-email-carlo@caione.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2/15/16 1:36 PM, Carlo Caione wrote: > From: Carlo Caione <carlo@endlessm.com> > > We cannot use strcpy() to write to a const char * location. This is > causing a 'BUG: unable to handle kernel paging request' error at boot > when using the cht-bsw-rt5645 driver. > > With this patch we also fix a wrong indexing in the driver where the > codec_name of the wrong dai_link is being overwritten. > > Signed-off-by: Carlo Caione <carlo@endlessm.com> > --- > sound/soc/intel/atom/sst-atom-controls.h | 1 + > sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/intel/atom/sst-atom-controls.h b/sound/soc/intel/atom/sst-atom-controls.h > index e011311..6f88d1c 100644 > --- a/sound/soc/intel/atom/sst-atom-controls.h > +++ b/sound/soc/intel/atom/sst-atom-controls.h > @@ -30,6 +30,7 @@ enum { > MERR_DPCM_AUDIO = 0, > MERR_DPCM_DEEP_BUFFER, > MERR_DPCM_COMPR, > + MERR_DPCM_BE, > }; > > /* define a bit for each mixer input */ > diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c > index e6cf800..89b4c032 100644 > --- a/sound/soc/intel/boards/cht_bsw_rt5645.c > +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c > @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { > }, > /* CODEC<->CODEC link */ > /* back ends */ > - { > + [MERR_DPCM_BE] = { > .name = "SSP2-Codec", > .be_id = 1, > .cpu_dai_name = "ssp2-port", > @@ -357,7 +357,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) > card->dev = &pdev->dev; > sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id); > /* set correct codec name */ > - strcpy((char *)card->dai_link[2].codec_name, codec_name); > + card->dai_link[MERR_DPCM_BE].codec_name = kstrdup(codec_name, GFP_KERNEL); It's probably less problematic to find the index in a different way. Adding a new definition in the platform driver doesn't seem quite right.
On Thu, Feb 18, 2016 at 9:02 AM, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote: > On 2/15/16 1:36 PM, Carlo Caione wrote: >> >> From: Carlo Caione <carlo@endlessm.com> >> >> We cannot use strcpy() to write to a const char * location. This is >> causing a 'BUG: unable to handle kernel paging request' error at boot >> when using the cht-bsw-rt5645 driver. >> >> With this patch we also fix a wrong indexing in the driver where the >> codec_name of the wrong dai_link is being overwritten. >> >> Signed-off-by: Carlo Caione <carlo@endlessm.com> >> --- >> sound/soc/intel/atom/sst-atom-controls.h | 1 + >> sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++-- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/sound/soc/intel/atom/sst-atom-controls.h >> b/sound/soc/intel/atom/sst-atom-controls.h >> index e011311..6f88d1c 100644 >> --- a/sound/soc/intel/atom/sst-atom-controls.h >> +++ b/sound/soc/intel/atom/sst-atom-controls.h >> @@ -30,6 +30,7 @@ enum { >> MERR_DPCM_AUDIO = 0, >> MERR_DPCM_DEEP_BUFFER, >> MERR_DPCM_COMPR, >> + MERR_DPCM_BE, >> }; >> >> /* define a bit for each mixer input */ >> diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c >> b/sound/soc/intel/boards/cht_bsw_rt5645.c >> index e6cf800..89b4c032 100644 >> --- a/sound/soc/intel/boards/cht_bsw_rt5645.c >> +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c >> @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { >> }, >> /* CODEC<->CODEC link */ >> /* back ends */ >> - { >> + [MERR_DPCM_BE] = { >> .name = "SSP2-Codec", >> .be_id = 1, >> .cpu_dai_name = "ssp2-port", >> @@ -357,7 +357,7 @@ static int snd_cht_mc_probe(struct platform_device >> *pdev) >> card->dev = &pdev->dev; >> sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id); >> /* set correct codec name */ >> - strcpy((char *)card->dai_link[2].codec_name, codec_name); >> + card->dai_link[MERR_DPCM_BE].codec_name = kstrdup(codec_name, >> GFP_KERNEL); > > > It's probably less problematic to find the index in a different way. Adding > a new definition in the platform driver doesn't seem quite right. MERR_DPCM_BE is intended to be the last dai link index corresponding to the BE dai. Or at least that was my idea. What about finding the index using a cycle and stopping on !strcmp(card->dai_link[i].codec_name, "i2c-10EC5645:00")?
On 02/18/2016 02:31 AM, Carlo Caione wrote: > On Thu, Feb 18, 2016 at 9:02 AM, Pierre-Louis Bossart > <pierre-louis.bossart@linux.intel.com> wrote: >> On 2/15/16 1:36 PM, Carlo Caione wrote: >>> >>> From: Carlo Caione <carlo@endlessm.com> >>> >>> We cannot use strcpy() to write to a const char * location. This is >>> causing a 'BUG: unable to handle kernel paging request' error at boot >>> when using the cht-bsw-rt5645 driver. >>> >>> With this patch we also fix a wrong indexing in the driver where the >>> codec_name of the wrong dai_link is being overwritten. >>> >>> Signed-off-by: Carlo Caione <carlo@endlessm.com> >>> --- >>> sound/soc/intel/atom/sst-atom-controls.h | 1 + >>> sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++-- >>> 2 files changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/sound/soc/intel/atom/sst-atom-controls.h >>> b/sound/soc/intel/atom/sst-atom-controls.h >>> index e011311..6f88d1c 100644 >>> --- a/sound/soc/intel/atom/sst-atom-controls.h >>> +++ b/sound/soc/intel/atom/sst-atom-controls.h >>> @@ -30,6 +30,7 @@ enum { >>> MERR_DPCM_AUDIO = 0, >>> MERR_DPCM_DEEP_BUFFER, >>> MERR_DPCM_COMPR, >>> + MERR_DPCM_BE, >>> }; >>> >>> /* define a bit for each mixer input */ >>> diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c >>> b/sound/soc/intel/boards/cht_bsw_rt5645.c >>> index e6cf800..89b4c032 100644 >>> --- a/sound/soc/intel/boards/cht_bsw_rt5645.c >>> +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c >>> @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { >>> }, >>> /* CODEC<->CODEC link */ >>> /* back ends */ >>> - { >>> + [MERR_DPCM_BE] = { >>> .name = "SSP2-Codec", >>> .be_id = 1, >>> .cpu_dai_name = "ssp2-port", >>> @@ -357,7 +357,7 @@ static int snd_cht_mc_probe(struct platform_device >>> *pdev) >>> card->dev = &pdev->dev; >>> sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id); >>> /* set correct codec name */ >>> - strcpy((char *)card->dai_link[2].codec_name, codec_name); >>> + card->dai_link[MERR_DPCM_BE].codec_name = kstrdup(codec_name, >>> GFP_KERNEL); >> >> >> It's probably less problematic to find the index in a different way. Adding >> a new definition in the platform driver doesn't seem quite right. > > MERR_DPCM_BE is intended to be the last dai link index corresponding > to the BE dai. Or at least that was my idea. > What about finding the index using a cycle and stopping one > !strcmp(card->dai_link[i].codec_name, "i2c-10EC5645:00")? Yes that was the idea. My concern is that if someone adds a front-end or conversely does not enable the link in the machine driver the index will be off. btw we have a fix coming to change the codec name, in some cases the BIOS is somewhat broken and the extension can be :01. If you go ahead with the loop I'll add the changes on top of yours in a follow-up patch. Thanks!
diff --git a/sound/soc/intel/atom/sst-atom-controls.h b/sound/soc/intel/atom/sst-atom-controls.h index e011311..6f88d1c 100644 --- a/sound/soc/intel/atom/sst-atom-controls.h +++ b/sound/soc/intel/atom/sst-atom-controls.h @@ -30,6 +30,7 @@ enum { MERR_DPCM_AUDIO = 0, MERR_DPCM_DEEP_BUFFER, MERR_DPCM_COMPR, + MERR_DPCM_BE, }; /* define a bit for each mixer input */ diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index e6cf800..89b4c032 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { }, /* CODEC<->CODEC link */ /* back ends */ - { + [MERR_DPCM_BE] = { .name = "SSP2-Codec", .be_id = 1, .cpu_dai_name = "ssp2-port", @@ -357,7 +357,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) card->dev = &pdev->dev; sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id); /* set correct codec name */ - strcpy((char *)card->dai_link[2].codec_name, codec_name); + card->dai_link[MERR_DPCM_BE].codec_name = kstrdup(codec_name, GFP_KERNEL); snd_soc_card_set_drvdata(card, drv); ret_val = devm_snd_soc_register_card(&pdev->dev, card); if (ret_val) {