From: Sam Protsenko <semen.protsenko@linaro.org> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Cc: Chanho Park <chanho61.park@samsung.com>, Jaewon Kim <jaewon02.kim@samsung.com>, Wolfram Sang <wsa@kernel.org>, Rob Herring <robh+dt@kernel.org>, linux-samsung-soc@vger.kernel.org, linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 2/2] i2c: exynos5: add support for ExynosAutov9 SoC Date: Thu, 18 Nov 2021 21:59:53 +0200 [thread overview] Message-ID: <CAPLW+4kS-pzROC5oyAjW1aJp5cb1e3XK+40HsKwgPdCziSp1ZQ@mail.gmail.com> (raw) In-Reply-To: <da9bd8cc-9415-6db7-024e-8d50b5f666f7@canonical.com> On Tue, 16 Nov 2021 at 11:32, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On 16/11/2021 02:12, Chanho Park wrote: > >> 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. > > > > 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. > > 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. > > > > 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. > > Option3) Each USI driver(uart/i2c/spi) has its own USI configurations respectively and expose some configurations which can be variable as device tree. > > > > [1]: https://github.com/ianmacd/d2s/blob/master/drivers/soc/samsung/usi_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. Thanks! > Best regards, > Krzysztof
WARNING: multiple messages have this Message-ID (diff)
From: Sam Protsenko <semen.protsenko@linaro.org> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Cc: Chanho Park <chanho61.park@samsung.com>, Jaewon Kim <jaewon02.kim@samsung.com>, Wolfram Sang <wsa@kernel.org>, Rob Herring <robh+dt@kernel.org>, linux-samsung-soc@vger.kernel.org, linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 2/2] i2c: exynos5: add support for ExynosAutov9 SoC Date: Thu, 18 Nov 2021 21:59:53 +0200 [thread overview] Message-ID: <CAPLW+4kS-pzROC5oyAjW1aJp5cb1e3XK+40HsKwgPdCziSp1ZQ@mail.gmail.com> (raw) In-Reply-To: <da9bd8cc-9415-6db7-024e-8d50b5f666f7@canonical.com> On Tue, 16 Nov 2021 at 11:32, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On 16/11/2021 02:12, Chanho Park wrote: > >> 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. > > > > 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. > > 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. > > > > 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. > > Option3) Each USI driver(uart/i2c/spi) has its own USI configurations respectively and expose some configurations which can be variable as device tree. > > > > [1]: https://github.com/ianmacd/d2s/blob/master/drivers/soc/samsung/usi_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. Thanks! > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-11-18 20:00 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20211112010603epcas2p331fe717eabfd9fc0280792921b25c535@epcas2p3.samsung.com> 2021-11-12 1:01 ` [PATCH v3 0/2] i2c: exynos5: add support for ExynosAutov9 SoC Jaewon Kim 2021-11-12 1:01 ` Jaewon Kim [not found] ` <CGME20211112010603epcas2p26c076e65e0cb286cb53f06053165ef60@epcas2p2.samsung.com> 2021-11-12 1:01 ` [PATCH v3 1/2] dt-bindings: i2c: exynos5: add exynosautov9-hsi2c compatible Jaewon Kim 2021-11-12 1:01 ` Jaewon Kim 2021-11-15 18:56 ` Sam Protsenko 2021-11-15 18:56 ` Sam Protsenko 2021-11-19 8:57 ` Krzysztof Kozlowski 2021-11-19 8:57 ` Krzysztof Kozlowski [not found] ` <CGME20211112010603epcas2p339d1a6ef3df7cdbe61c87c8afa541fd0@epcas2p3.samsung.com> 2021-11-12 1:01 ` [PATCH v3 2/2] i2c: exynos5: add support for ExynosAutov9 SoC Jaewon Kim 2021-11-12 1:01 ` Jaewon Kim 2021-11-12 8:09 ` Krzysztof Kozlowski 2021-11-12 8:09 ` Krzysztof Kozlowski 2021-11-15 18:55 ` Sam Protsenko 2021-11-15 18:55 ` Sam Protsenko 2021-11-16 1:12 ` Chanho Park 2021-11-16 1:12 ` Chanho Park 2021-11-16 9:31 ` Krzysztof Kozlowski 2021-11-16 9:31 ` Krzysztof Kozlowski 2021-11-16 15:31 ` Sam Protsenko 2021-11-16 15:31 ` Sam Protsenko 2021-11-19 8:51 ` Krzysztof Kozlowski 2021-11-19 8:51 ` Krzysztof Kozlowski 2021-11-18 19:59 ` Sam Protsenko [this message] 2021-11-18 19:59 ` Sam Protsenko 2021-11-19 8:54 ` Krzysztof Kozlowski 2021-11-19 8:54 ` Krzysztof Kozlowski 2021-11-19 14:12 ` Sam Protsenko 2021-11-19 14:12 ` Sam Protsenko 2021-11-22 2:51 ` Jaewon Kim 2021-11-22 2:51 ` Jaewon Kim 2021-11-17 22:17 ` David Virag 2021-11-17 22:17 ` David Virag
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAPLW+4kS-pzROC5oyAjW1aJp5cb1e3XK+40HsKwgPdCziSp1ZQ@mail.gmail.com \ --to=semen.protsenko@linaro.org \ --cc=chanho61.park@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=jaewon02.kim@samsung.com \ --cc=krzysztof.kozlowski@canonical.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-i2c@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=wsa@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.