On 2014/5/16 22:45, Andy Shevchenko wrote: > 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. That's exactly what I worry about. It's very hard to figure it out. > >> 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. Is it better to rephrase the help information to give some hints to the user? Thanks, -Aubrey > >> >> 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. > > -- 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
