On 03/12/2017, 06:20 AM, Greg KH wrote:
> On Sat, Mar 11, 2017 at 04:28:53PM +0100, Jiri Slaby wrote:
>> On 03/08/2017, 11:27 AM, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
>>> From 15c75b09f8d190f89ab4db463b87d411ca349dfe Mon Sep 17 00:00:00 2001
>>> From: Takashi Iwai <tiwai@xxxxxxx>
>>> Date: Tue, 28 Feb 2017 17:16:48 +0100
>>> Subject: ALSA: ctxfi: Fallback DMA mask to 32bit
>>>
>>> From: Takashi Iwai <tiwai@xxxxxxx>
>>>
>>> commit 15c75b09f8d190f89ab4db463b87d411ca349dfe upstream.
>>>
>>> Currently ctxfi driver tries to set only the 64bit DMA mask on 64bit
>>> architectures, and bails out if it fails. This causes a problem on
>>> some platforms since the 64bit DMA isn't always guaranteed. We should
>>> fall back to the default 32bit DMA when 64bit DMA fails.
>>>
>>> Fixes: 6d74b86d3c0f ("ALSA: ctxfi - Allow 64bit DMA")
>>> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> ...
>>> @@ -1904,19 +1898,18 @@ static int hw_card_start(struct hw *hw)
>>> {
>>> int err;
>>> struct pci_dev *pci = hw->pci;
>>> + const unsigned int dma_bits = BITS_PER_LONG;
>>>
>>> err = pci_enable_device(pci);
>>> if (err < 0)
>>> return err;
>>>
>>> /* Set DMA transfer mask */
>>> - if (dma_set_mask(&pci->dev, CT_XFI_DMA_MASK) < 0 ||
>>> - dma_set_coherent_mask(&pci->dev, CT_XFI_DMA_MASK) < 0) {
>>> - dev_err(hw->card->dev,
>>> - "architecture does not support PCI busmaster DMA with mask 0x%llx\n",
>>> - CT_XFI_DMA_MASK);
>>> - err = -ENXIO;
>>> - goto error1;
>>> + if (dma_set_mask(&pci->dev, DMA_BIT_MASK(dma_bits))) {
>>
>> I believe the condition is inverted here.
>>
>>> + dma_set_coherent_mask(&pci->dev, DMA_BIT_MASK(dma_bits));
>>> + } else {
>>> + dma_set_mask(&pci->dev, DMA_BIT_MASK(32));
>>> + dma_set_coherent_mask(&pci->dev, DMA_BIT_MASK(32));
>
> As this matches what is in Linus's tree, that means I didn't mess up the
> backport :)
Sure, I didn't mean to point out your failure, rather Takashi wrote the
check wrong :). I found it in 4.4 patches while applying them, so I
replied to that particular one.
> But I don't understand the issue, what is wrong here? Should it try 32
> first?
It should be:
dma_set_mask(dma_bits) fails => fall back to 32 bits
dma_set_mask(dma_bits) does not fail => set also the coherent mask
As it stands now, it is the opposite.
> Takashi, any thoughts?
IMO, it should write:
> if (!dma_set_mask(&pci->dev, DMA_BIT_MASK(dma_bits))) {
> dma_set_coherent_mask(&pci->dev, DMA_BIT_MASK(dma_bits));
thanks,
--
js
suse labs