Hi Protsenko,

On Fri, 19 Nov 2021 at 10:54, Krzysztof Kozlowski [off-list ref] wrote:

quoted

On 18/11/2021 20:59, Sam Protsenko wrote:

quoted

On Tue, 16 Nov 2021 at 11:32, Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 16/11/2021 02:12, Chanho Park wrote:

quoted

quoted

With this patch the Exynos850 HSI2C becomes functional. The only
nit-pick from my side (just a food for thought): do we want to
configure USI related config inside of particular drivers (SPI,
I2C, UART)? Or it would be better design to implement some
platform driver for that, so we can choose USI configuration
(SPI/I2C/UART) in device tree? I think this series is good to be
merged as is, but we should probably consider all upsides and downsides of each option, for the

future work.

quoted

quoted

quoted

quoted

I'm also considering how to support this USI configuration gracefully.
Current version of USI is v2 which means there is a v1 version as well. It might be a non-

upstream SoC so we don't need to consider it so far.

quoted

quoted

quoted

quoted

But, there is a possibility that the USI hw version can be bumped for future SoCs.

As you probably know, earlier version of the product kernel has a USI SoC driver[1] and it was

designed to be configured the USI settings by device tree.

quoted

quoted

quoted

quoted

Option1) Make a USI driver under soc/samsung/ like [1].
Option2) Use more generic driver such as "reset driver"? This might be required to extend the

reset core driver.

quoted

quoted

quoted

quoted

Option3) Each USI driver(uart/i2c/spi) has its own USI configurations respectively and expose

some configurations which can be variable as device tree.

quoted

quoted

quoted

quoted

[1]:
https://protect2.fireeye.com/v1/url?k=b290a67b-ed0b9f6a-b2912d34-0
cc47a31cdbc-ceadd8e62313162a&q=1&e=317825c0-3fac-46ad-9b4e-f93de42
ad5ba&u=https%3A%2F%2Fgithub.com%2Fianmacd%2Fd2s%2Fblob%2Fmaster%2
Fdrivers%2Fsoc%2Fsamsung%2Fusi_v2.c

I don't have user manuals, so all my knowledge here is based on
Exynos9825 vendor source code, therefore it is quite limited. In
devicetree the USI devices have their own nodes - but does it mean
it's separate SFR range dedicated to USI? Looks like that,
especially that address space is just for one register (4 bytes).

In such case having separate dedicated driver makes sense and you
would only have to care about driver ordering (e.g. via device links or phandles).

Option 2 looks interesting - reusing reset framework to set proper
USI mode, however this looks more like a hack. As you said Chanho,
if there is a USI version 3, this reset framework might not be sufficient.

In option 3 each driver (UART/I2C/SPI) would need to receive second
IO range and toggle some registers, which could be done via shared
function. If USI v3 is coming, all such drivers could get more complicated.

I think option 1 is the cleanest and extendable in future. It's
easy to add usi-v3 or whatever without modifying the UART/I2C/SPI
drivers. It also nicely encapsulates USI-related stuff in separate
driver. Probe ordering should not be a problem now.

But as I said, I don't have even the big picture here, so I rely on
your opinions more.

Hi Krzysztof,

Can you please let me know if you're going to apply this series as
is, or if you want me to submit USIv2 driver first, and then rework
this patch on top of it? I'm working on some HSI2C related patches
right now, and thus it'd nice to know about your decision on this
series beforehand, as some of my patches (like bindings doc patches)
might depend on it. Basically I'd like to base my patches on the
proper baseline, so we don't have to rebase those later.

This set won't go via my tree anyway, but I am against it. David
pointed out that his USIv1 is a little bit different and embedding in
each of I2C/UART/SPI drivers the logic of controlling USIv1 and USIv2
looks too big. The solution with a dedicated driver looks to me more
flexible and encapsulated/cleaner.

Therefore after the discussions I am against this solution, so a
soft-NAK from my side.

Hi Jaewon,

I'm going to submit USI driver soon, and also some more HSI2C patches.
Do you mind if I rework your patches to rely on USI drver (instead of modifying System Register in
HSI2C driver), and include those in my patch series? Of course, I'll preserve your authorship. Just
think that would be easier and faster this way.

Thanks!

I'm glad you're working on a USI driver.
You can use my patchset.

quoted

Best regards,
Krzysztof

Thanks
Jaewon Kim


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel