Re: [PATCH 2/2] ACPI / scan: Fix enumeration for special UART devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johan,

>>>>>> UART devices is expected to be enumerated by SerDev subsystem.
>>>>>> 
>>>>>> During ACPI scan, serial devices behind SPI, I2C or UART buses are not
>>>>>> enumerated, allowing them to be enumerated by their respective parents.
>>>>>> 
>>>>>> Rename *spi_i2c_slave* to *serial_bus_slave* as this will be used for serial
>>>>>> devices on serial buses (SPI, I2C or UART).
>>>>>> 
>>>>>> On Macs an empty ResourceTemplate is returned for uart slaves.
>>>>>> Instead the device properties "baud", "parity", "dataBits", "stopBits" are
>>>>>> provided. Add a check for "baud" in acpi_is_serial_bus_slave().
>>>>>> 
>>>>>> Signed-off-by: Frédéric Danis <frederic.danis.oss@xxxxxxxxx>
>>>>> 
>>>>> So just to reiterate what I just mentioned in a comment to one of Hans's
>>>>> hci_bcm patches:
>>>>> 
>>>>> This one would silently break PM for such devices on any system which
>>>>> does not have serdev enabled (as the corresponding platform devices
>>>>> would no longer be registered). And with serdev enabled, hciattach
>>>>> (btattach) would start failing as the tty device would no longer be
>>>>> registered (but I assume everyone is aware of that, and fine with it, by
>>>>> now).
>>>>> 
>>>>> Perhaps the hci_bcm driver should start depending on
>>>>> SERIAL_DEV_CTRL_TTYPORT when ACPI is enabled?
>>>> 
>>>> ACPI and DT both need SERIAL_DEV_CTRL_TTYPORT to work properly,
>>>> since SERIAL_DEV_CTRL_TTYPORT is the only controller implemented
>>>> for serdev. If any other controller is implemented that one could
>>>> also be used.
>>> 
>>> Not for hci_bcm, right? This particular driver specifically depends on
>>> SERIAL_DEV_CTRL_TTYPORT for the ACPI devices and not just any (future)
>>> serdev controller (or currently working systems soon breaks silently).
>>> 
>>> I don't think the same is true for the DT case where we do not already
>>> have child nodes defined in firmware (and in fact, this driver did not
>>> really support DT before serdev).
>> 
>> The serdev ACPI support has been added to the core and not to
>> the ttyport and the hci_bcm driver only uses functions from the
>> core. As far as I can see the ACPI part would also work fine with
>> a different serdev controller.
> 
> Indeed, but you need SERIAL_DEV_CTRL_TTYPORT to avoid silently breaking
> current ACPI setups which breaks when this patch is applied (as these
> devices all hang off of common serial ports managed by serial core).
> 
>> Of course DT and ACPI currently require SERIAL_DEV_CTRL_TTYPORT,
>> since it's the only serdev controller implementation. Also it
>> covers most use cases. When SERIAL_DEV_BUS is selected it's
>> very likely, that you also want SERIAL_DEV_CTRL_TTYPORT.
> 
>>>> I wonder if we should just hide SERIAL_DEV_CTRL_TTYPORT and enable
>>>> it together with SERDEV. I suspect that we won't see any other
>>>> controller (it would be a UART device, that is not registered as
>>>> tty device) in the next few years and the extra option seems to
>>>> confuse people.
>>> 
>>> I agree that it is somewhat confusing. But now that we have both,
>>> perhaps simply having SERIAL_DEV_CTRL_TTYPORT default to "y" when
>>> SERIAL_DEV_BUS is selected could be a compromise. The Kconfig entry
>>> might need to be amended as well (e.g. if only to mention that you
>>> need to select a controller as well).
>> 
>> I think we should at least add a default "y" if SERIAL_DEV_BUS.
> 
> I'm preparing a patch.

yes, please prepare a patch since the discussion spans multiple email threads now. Lets get this back on track and find a patch that we are all happy with.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux