On Tue, Nov 20, 2018 at 02:38:32PM +0800, Ming Huang wrote:
> > My feedback was:
> > ---
> > This would be more clear as "platform specific" than "cpld relative".
> > 
> > I did not realise this wasn't a Hisilicon component when reviewing the
> > original set.
> > 
> > I approve of this change, but can you tell me why it is included in
> > this set? If the goal is to make the M41T83 support platform
> > independent, should the library also move to Silicon/ST/?
> > ---
> > 
> > So could you please update the commit message, and add a subsequent
> > patch moving Library/M41T83RealTimeClockLib to
> > Silicon/STMicroelectronics (and updating D06.dsc to match)?
> > I do not care if it is not perfectly abstracted yet - we can deal with
> > that when we have other users of the component in the tree.
> 
> Sorry for missing update the commit message this patch.
> I will update it in v3. I try to move the library to 
> Silicon/STMicroelectronics,
> but M41T83RealTimeClockLib depend on I2CLib in Hisilicon, so can't move the
> library to STMicroelectronics. Main gist of this patch
> is making this library as a common module in Hisilicon.

And a PCI component depends on PciLib - that does not mean it should
live in edk2 MdePkg. And it does not mean that this library should be
considered Hisilicon-specific. If we get other platforms with this
component, they will not be permitted to add duplicated code - we
will then need to make sure the library is suitable for all users.

There is no situation other than the maintainers (me) dropping the
ball that lets you have a private driver for a generic component.

Regardless, I will leave this patch out for now, to avoid delaying the
release.

/
    Leif
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to