On Fri, 2014-05-16 at 21:37 +0800, Li, Aubrey wrote: > On 2014/5/16 15:04, Andy Shevchenko wrote: > > On Fri, 2014-05-16 at 07:29 +0800, Li, Aubrey wrote: > >> On 2014/5/16 0:11, Mika Westerberg wrote: > >>> On Thu, May 15, 2014 at 11:59:46PM +0800, Li, Aubrey wrote: > >>>> On 2014/5/15 22:53, Andy Shevchenko wrote: > >>>>> On Thu, 2014-05-15 at 22:35 +0800, Li, Aubrey wrote: > >>>>>> On 2014/5/15 21:40, Heikki Krogerus wrote: > >>>>>>> Changes since v1: > >>>>>>> - now using do_div() in clk_fd_recalc_rate() as suggested by Andy > >>>>>>> - NULL checks for clk_name allocation in acpi_lpss.c > >>>>>>> > >>>>>>> This combines two patch sets for LPSS that I had already send for > >>>>>>> review separately. They conflicted with each other. > >>>>>>> > >>>>>>> The first two patches will fix a problem were the context of the > >>>>>>> private LPSS registers is lost when entering D3. The last two will add > >>>>>>> support for the M/N dividers on LPSS by adding a new basic clock type > >>>>>>> for fractional dividers. The UART driver needs support for it in order > >>>>>>> to get clock rates that suit the requested baud rates. > >>>>>> > >>>>>> The major issue in my mind is, this proposal makes a couple between I2C > >>>>>> designware, HSUART, or probably DMA driver as well with LPSS driver. > >>>>> > >>>>> acpi_lpss driver creates platform devices for each found and enumerated > >>>>> device. > >>>> > >>>>> If there no acpi_lpss enabled the drivers work as supposed without it. > >>>> > >>>> This is not true. > >>> > >>> The drivers work fine on non-LPSS platform. If you make them depend on > >>> acpi_lpss, you break that. > >>> > >> > >> People don't know the relationship between LPSS driver and I2C/HSUART, > >> there is nowhere to describe that. If LPSS driver is unchecked, they > >> will encounter a weird issue which is very hard to figure out what's > >> going on. > > > > Besides this discussion is off the topic, I could say that LPSS drivers > > are kinda optional (we won't enforce user to use them) even on some > > systems where they are present. Relationship is provided by the proper > > kernel configuration. > > > It is optional previously but definitely not optional after this patch. I'm sorry, I didn't clearly see what part in this patchset prevents this. > The user who uses I2C designeware driver without LPSS now, I2C won't > work properly on Asus T100. In that case you specifically enable this driver in the kernel configuration. T100 may require that driver to be fully functional. > > The proper configuration leads to a question, why a completed I2C > controller driver doesn't work properly without another LPSS driver. I > worry about this is hard to maintain in future. > > Do we have a platform configuration to specify LPSS is needed? > > BTW, do we have a system with I2C and HSUART but without LPSS? I believe we have. -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html