Message ID | 20140227011300.8CD451983B@mono.eik.bme.hu |
---|---|
State | New |
Headers | show |
Ping! http://patchwork.ozlabs.org/patch/324674/ On Thu, 27 Feb 2014, BALATON Zoltan wrote: > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/char/serial-pci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c > index 991c99f..e662b77 100644 > --- a/hw/char/serial-pci.c > +++ b/hw/char/serial-pci.c > @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) > return -1; > } > > + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; > s->irq = pci_allocate_irq(&pci->dev); > > -- > 1.8.1.5
10.03.2014 22:40, BALATON Zoltan wrote: > Ping! > http://patchwork.ozlabs.org/patch/324674/ > > On Thu, 27 Feb 2014, BALATON Zoltan wrote: >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/char/serial-pci.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c >> index 991c99f..e662b77 100644 >> --- a/hw/char/serial-pci.c >> +++ b/hw/char/serial-pci.c >> @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) >> return -1; >> } >> >> + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ >> pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; >> s->irq = pci_allocate_irq(&pci->dev); Hm. While this is an one-liner indeed, I'm not sure it's the right value. From my pci.ids: C 07 Communication controller 00 Serial controller 00 8250 01 16450 02 16550 03 16650 04 16750 05 16850 06 16950 so it seems correct thing to do. What about multi_serial_pci_init() -- should it too set prog-if like this? Thanks, /mjt
On Fri, 14 Mar 2014, Michael Tokarev wrote: > 10.03.2014 22:40, BALATON Zoltan wrote: >> Ping! >> http://patchwork.ozlabs.org/patch/324674/ >> >> On Thu, 27 Feb 2014, BALATON Zoltan wrote: >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> hw/char/serial-pci.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c >>> index 991c99f..e662b77 100644 >>> --- a/hw/char/serial-pci.c >>> +++ b/hw/char/serial-pci.c >>> @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) >>> return -1; >>> } >>> >>> + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ >>> pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; >>> s->irq = pci_allocate_irq(&pci->dev); > > Hm. While this is an one-liner indeed, I'm not sure it's the right > value. From my pci.ids: > > C 07 Communication controller > 00 Serial controller > 00 8250 > 01 16450 > 02 16550 > 03 16650 > 04 16750 > 05 16850 > 06 16950 > > so it seems correct thing to do. > > What about multi_serial_pci_init() -- should it too set prog-if > like this? I'm not completely sure what the correct value should be for multi_serial_pci_init so I've left that alone. There's also 07 02 00 that means multiport serial controller that might apply for that case. For the simple case I've sent the patch for I had a client expecting this value so I think it is correct for that at least but I don't know about the multiport case. Regards, BALATON Zoltan
On Fri, 14 Mar 2014, BALATON Zoltan wrote: > On Fri, 14 Mar 2014, Michael Tokarev wrote: >> 10.03.2014 22:40, BALATON Zoltan wrote: >>> Ping! >>> http://patchwork.ozlabs.org/patch/324674/ >>> >>> On Thu, 27 Feb 2014, BALATON Zoltan wrote: >>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>>> --- >>>> hw/char/serial-pci.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c >>>> index 991c99f..e662b77 100644 >>>> --- a/hw/char/serial-pci.c >>>> +++ b/hw/char/serial-pci.c >>>> @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) >>>> return -1; >>>> } >>>> >>>> + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ >>>> pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; >>>> s->irq = pci_allocate_irq(&pci->dev); >> >> Hm. While this is an one-liner indeed, I'm not sure it's the right >> value. From my pci.ids: >> >> C 07 Communication controller >> 00 Serial controller >> 00 8250 >> 01 16450 >> 02 16550 >> 03 16650 >> 04 16750 >> 05 16850 >> 06 16950 >> >> so it seems correct thing to do. >> >> What about multi_serial_pci_init() -- should it too set prog-if >> like this? > > I'm not completely sure what the correct value should be for > multi_serial_pci_init so I've left that alone. There's also 07 02 00 that > means multiport serial controller that might apply for that case. For the > simple case I've sent the patch for I had a client expecting this value so I > think it is correct for that at least but I don't know about the multiport > case. Also Gerd Hoffmann said he has picked this patch up so I'm adding him to cc too. > Regards, > BALATON Zoltan
> >> C 07 Communication controller > >> 00 Serial controller > >> 00 8250 > >> 01 16450 > >> 02 16550 > >> What about multi_serial_pci_init() -- should it too set prog-if > >> like this? > > > > I'm not completely sure what the correct value should be for > > multi_serial_pci_init so I've left that alone. There's also 07 02 00 that > > means multiport serial controller that might apply for that case. For the > > simple case I've sent the patch for I had a client expecting this value so I > > think it is correct for that at least but I don't know about the multiport > > case. > > Also Gerd Hoffmann said he has picked this patch up so I'm adding him to > cc too. The multiport variants are simply multiple 16550 chips mapped one after the other. So "02" is better than "00". I don't know what exactly the multiport serial controller is supposed to be. cheers, Gerd
On Mon, 17 Mar 2014, Gerd Hoffmann wrote: >>>> C 07 Communication controller >>>> 00 Serial controller >>>> 00 8250 >>>> 01 16450 >>>> 02 16550 > >>>> What about multi_serial_pci_init() -- should it too set prog-if >>>> like this? >>> >>> I'm not completely sure what the correct value should be for >>> multi_serial_pci_init so I've left that alone. There's also 07 02 00 that >>> means multiport serial controller that might apply for that case. For the >>> simple case I've sent the patch for I had a client expecting this value so I >>> think it is correct for that at least but I don't know about the multiport >>> case. >> >> Also Gerd Hoffmann said he has picked this patch up so I'm adding him to >> cc too. > > The multiport variants are simply multiple 16550 chips mapped one after > the other. So "02" is better than "00". > > I don't know what exactly the multiport serial controller is supposed to > be. So what should we do then? Should I send an updated patch which sets 070002 in the multiport case too? Regards, BALATON Zoltan
10.03.2014 22:40, BALATON Zoltan wrote: > Ping! > http://patchwork.ozlabs.org/patch/324674/ Thanks, applied to -trivial. /mjt
Il 20/03/2014 16:42, Michael Tokarev ha scritto: > 10.03.2014 22:40, BALATON Zoltan wrote: >> Ping! >> http://patchwork.ozlabs.org/patch/324674/ > > Thanks, applied to -trivial. No, please don't; this needs to be done for new machine types only. Paolo
On Thu, 20 Mar 2014, Paolo Bonzini wrote: > Il 20/03/2014 16:42, Michael Tokarev ha scritto: >> 10.03.2014 22:40, BALATON Zoltan wrote: >>> Ping! >>> http://patchwork.ozlabs.org/patch/324674/ >> >> Thanks, applied to -trivial. > > No, please don't; this needs to be done for new machine types only. Why? I tried running a Windows XP image that was set up before this patch and it did not complain about changed hardware. Or is there another reason I'm missing? Regards, BALATON Zoltan
Il 20/03/2014 17:01, BALATON Zoltan ha scritto: > On Thu, 20 Mar 2014, Paolo Bonzini wrote: >> Il 20/03/2014 16:42, Michael Tokarev ha scritto: >>> 10.03.2014 22:40, BALATON Zoltan wrote: >>>> Ping! >>>> http://patchwork.ozlabs.org/patch/324674/ >>> >>> Thanks, applied to -trivial. >> >> No, please don't; this needs to be done for new machine types only. > > Why? I tried running a Windows XP image that was set up before this > patch and it did not complain about changed hardware. Or is there > another reason I'm missing? Classes (or anything guest-visible in fact) must never ever be changed for the same machine type. It's a rule. Paolo
On Thu, 20 Mar 2014, Paolo Bonzini wrote: > Il 20/03/2014 17:01, BALATON Zoltan ha scritto: >> On Thu, 20 Mar 2014, Paolo Bonzini wrote: >>> Il 20/03/2014 16:42, Michael Tokarev ha scritto: >>>> 10.03.2014 22:40, BALATON Zoltan wrote: >>>>> Ping! >>>>> http://patchwork.ozlabs.org/patch/324674/ >>>> >>>> Thanks, applied to -trivial. >>> >>> No, please don't; this needs to be done for new machine types only. >> >> Why? I tried running a Windows XP image that was set up before this >> patch and it did not complain about changed hardware. Or is there >> another reason I'm missing? > > Classes (or anything guest-visible in fact) must never ever be changed for > the same machine type. It's a rule. The class did not change only the prog interface field after it (which is guest visible but I think most guests just ignore it). So then what's the correct way to do it? Do I need to change the patch or can it be merged as it is the next time a new machine type is added? Regards, BALATON Zoltan
Il 20/03/2014 17:58, BALATON Zoltan ha scritto: >> >> Classes (or anything guest-visible in fact) must never ever be changed >> for the same machine type. It's a rule. > > The class did not change only the prog interface field after it (which > is guest visible but I think most guests just ignore it). So then what's > the correct way to do it? Do I need to change the patch or can it be > merged as it is the next time a new machine type is added? You can wait for the 2.1 machine type to be added. Paolo
20.03.2014 19:51, Paolo Bonzini wrote: > Il 20/03/2014 16:42, Michael Tokarev ha scritto: >> 10.03.2014 22:40, BALATON Zoltan wrote: >>> Ping! >>> http://patchwork.ozlabs.org/patch/324674/ >> >> Thanks, applied to -trivial. > > No, please don't; this needs to be done for new machine types only. I thought about this too, but decided this is too minor a difference to warrant special per-machine-type handling. Oh well. /mjt
On Do, 2014-03-20 at 10:58 +0100, BALATON Zoltan wrote: > So what should we do then? Should I send an updated patch which sets > 070002 in the multiport case too? Yes, that would be great. cheers, Gerd
On Fri, 21 Mar 2014, Gerd Hoffmann wrote: > On Do, 2014-03-20 at 10:58 +0100, BALATON Zoltan wrote: >> So what should we do then? Should I send an updated patch which sets >> 070002 in the multiport case too? > > Yes, that would be great. OK, I'll resubmit it after pc-2.1 is added also with the multiport case. Thank you, BALATON Zoltan
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c index 991c99f..e662b77 100644 --- a/hw/char/serial-pci.c +++ b/hw/char/serial-pci.c @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) return -1; } + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; s->irq = pci_allocate_irq(&pci->dev);
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/char/serial-pci.c | 1 + 1 file changed, 1 insertion(+)