Re: [PATCH v4] scsi: ufs: add support of generic PHY and ICE in Qualcomm chips

From: Christoph Hellwig
Date: Thu Dec 04 2014 - 10:54:58 EST


On Thu, Nov 27, 2014 at 05:59:58PM +0200, Yaniv Gardi wrote:
> In this change we add support to the generic PHY framework.
> Two UFS phys are implemented:
> qmp-20nm and qmp-28nm.
>
> Also, the files in this change implement the UFS HW (controller & PHY)
> specific behavior in Qualcomm chips.
> Relocation of a few header files is needed in order to expose routines
> and data structures between PHY driver and UFS driver.
>
> Also, this change include the implementation of Inline Crypto Engine (ICE)
> in Qualcomm chips.

This whole patch is a mess. It does way to many things in one patch,
and it doesn't explain enough of it.

Please explain why you need it. Especially as the PHY API is a generic
phy abstraction, so having to share defintions between the provider and
consumer seems wrong. Even if you need some shared bits keep them to an
absolute minium insted of moving so much out of the driver directory.
Also if at all possible keep the shared data in a single header under
include/linux instead of having lots of global headers in a deep
directory structure.

Second split this into patches that do a single things, and explain why
you're doing each:

1) header move if/as needed
2) add 20nm phy driver
3) add 28nm phy driver
4) add ufs-qcom driver
5) add ufs-qcom-ice support

and so on.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/