Salutations, Linux I2C team!
Sorry about the resend; it seems my mail client "helpfully" swallowed the
newlines on any line consisting only of whitespace, garbling the patches.
I am working with an Allwinner T113-s3 based board; the internal I2C bus of
which has a Realtek RTL8370MB-CG Ethernet switch coexisting with other I2C
devices. The RTL8370MB-CG deviates from "conventional" I2C read operations
in that it expects the hardware register after the addr+read byte before it
turns around the bus to send the value. For this reason, the `realtek-smi`
driver currently implements the protocol via bit-banging. However, I am in the
process of developing a separate patch series to promote this driver to a
"full" I2C driver, leveraging I2C_M_NOSTART to support this odd read operation.
In anticipation of that, I am preparing this series comprising five patches to
improve the functionality and reliability of the I2C adapter enough to support
this kind of device. I have heavily tested these changes on the Allwinner-style
mv64xxx core, but not the Marvell-style, and have not been able to test 10-bit
addressing. I would greatly appreciate if anyone here could test this series,
especially on non-Allwinner boards and/or boards with 10-bit devices.
I'm a bit skeptical of using I2C_M_NOSTART for this purpose. The driver does
not (and cannot) support "just any" use of I2C_M_NOSTART, so it may be
inappropriate to claim the I2C_FUNC_NOSTART capability. On the other hand, I
searched high and low and couldn't find any use of I2C_M_NOSTART that
*wouldn't* be supported by this change, so this could very well be exactly what
clients understand I2C_FUNC_NOSTART to mean. Given that the alternative would
be inventing a new flag ("I2C_M_READEXTRA"?) and figuring out how to supply
input bytes and output bytes in the same i2c_msg, I opted for the NOSTART
route instead.
I look forward to any feedback, bug reports, test results, questions, concerns,
commentary, or discussion that you can offer!
Best regards,
Sam
Sam Edwards (5):
i2c: mv64xxx: Clear bus errors before transfer
i2c: mv64xxx: Clean up the private data struct
i2c: mv64xxx: Refactor FSM
i2c: mv64xxx: Allow continuing after read
i2c: mv64xxx: Implement I2C_FUNC_NOSTART
drivers/i2c/busses/i2c-mv64xxx.c | 430 ++++++++++++++++++++++---------
1 file changed, 302 insertions(+), 128 deletions(-)
--
2.43.2
Hi Sam,
Thanks for your patches.
> Sorry about the resend; it seems my mail client "helpfully" swallowed the
> newlines on any line consisting only of whitespace, garbling the patches.
I received three series from you:
1. [RESEND v2 RFC 1/5] i2c: mv64xxx: Clear bus errors before transfer
2. [RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
3. [RESEND RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
By the versioning, 1. is good, the rest is not good. Standing to
the time sent and comments in patch '0', 3. is good, the rest
not.
Which one should be discarded? Can you please state it clearly?
Besides, youre mails are not threaded, They look like:
Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 1/5] i2c: mv64xxx: Clear bus errors before transfer
Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 2/5] i2c: mv64xxx: Clean up the private data struct
Mar 19 Sam Edwards ( 15K) [RESEND RFC PATCH 3/5] i2c: mv64xxx: Refactor FSM
Mar 19 Sam Edwards (5.2K) [RESEND RFC PATCH 4/5] i2c: mv64xxx: Allow continuing after read
Mar 19 Sam Edwards ( 11K) [RESEND RFC PATCH 5/5] i2c: mv64xxx: Implement I2C_FUNC_NOSTART
instead of
Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
Mar 19 Sam Edwards (2.3K) ├─>[RESEND RFC PATCH 1/5] i2c: mv64xxx: Clear bus errors before transfer
Mar 19 Sam Edwards (2.3K) ├─>[RESEND RFC PATCH 2/5] i2c: mv64xxx: Clean up the private data struct
Mar 19 Sam Edwards ( 15K) ├─>[RESEND RFC PATCH 3/5] i2c: mv64xxx: Refactor FSM
Mar 19 Sam Edwards (5.2K) ├─>[RESEND RFC PATCH 4/5] i2c: mv64xxx: Allow continuing after read
Mar 19 Sam Edwards ( 11K) └─>[RESEND RFC PATCH 5/5] i2c: mv64xxx: Implement I2C_FUNC_NOSTART
Which is the default of "git format-patch".
Can you please make sure, next time (unless someone asks to
resend them again), that the patches are threaded? You can send
them to yourself first and see if they are really threaded.
If you are using some weird mail client, you can also check the
mail header, making sure that mails from 1 to 5 have the field:
In-Reply-To: <Message-Id of patch 0>
Andi
On 3/20/24 20:28, Andi Shyti wrote:
> Hi Sam,
>
> Thanks for your patches.
>
>> Sorry about the resend; it seems my mail client "helpfully" swallowed the
>> newlines on any line consisting only of whitespace, garbling the patches.
>
> I received three series from you:
>
> 1. [RESEND v2 RFC 1/5] i2c: mv64xxx: Clear bus errors before transfer
> 2. [RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
> 3. [RESEND RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
>
> By the versioning, 1. is good, the rest is not good. Standing to
> the time sent and comments in patch '0', 3. is good, the rest
> not.
>
> Which one should be discarded? Can you please state it clearly?
Hi Andi,
Thanks so much for your patience -- this is the first series I sent with
this particular mail client and it really didn't go as smoothly as a
plain git-send-email. I'll get it nailed down in time for the non-RFC
series.
I sent the series in the order 2-3-1, so 1 is the version to look at
(though I made no content changes between resends as I was only fighting
my mail client's formatting).
>
> Besides, youre mails are not threaded, They look like:
>
> Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
> Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 1/5] i2c: mv64xxx: Clear bus errors before transfer
> Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 2/5] i2c: mv64xxx: Clean up the private data struct
> Mar 19 Sam Edwards ( 15K) [RESEND RFC PATCH 3/5] i2c: mv64xxx: Refactor FSM
> Mar 19 Sam Edwards (5.2K) [RESEND RFC PATCH 4/5] i2c: mv64xxx: Allow continuing after read
> Mar 19 Sam Edwards ( 11K) [RESEND RFC PATCH 5/5] i2c: mv64xxx: Implement I2C_FUNC_NOSTART
>
> instead of
>
> Mar 19 Sam Edwards (2.3K) [RESEND RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
> Mar 19 Sam Edwards (2.3K) ├─>[RESEND RFC PATCH 1/5] i2c: mv64xxx: Clear bus errors before transfer
> Mar 19 Sam Edwards (2.3K) ├─>[RESEND RFC PATCH 2/5] i2c: mv64xxx: Clean up the private data struct
> Mar 19 Sam Edwards ( 15K) ├─>[RESEND RFC PATCH 3/5] i2c: mv64xxx: Refactor FSM
> Mar 19 Sam Edwards (5.2K) ├─>[RESEND RFC PATCH 4/5] i2c: mv64xxx: Allow continuing after read
> Mar 19 Sam Edwards ( 11K) └─>[RESEND RFC PATCH 5/5] i2c: mv64xxx: Implement I2C_FUNC_NOSTART
>
> Which is the default of "git format-patch".
>
> Can you please make sure, next time (unless someone asks to
> resend them again), that the patches are threaded? You can send
> them to yourself first and see if they are really threaded.
Yes, definitely. I take it from your phrasing that you're willing to
collect the scattered mails yourself this one time only? If so, thank
you for cleaning up after my mess. :)
If not (and/or if someone else doesn't like the mess), I can always
resend. I have already made one cleanup (removing the useless `default:`
at the end of the FSM) so I guess it would technically be an "RFC v2" at
this point.
> If you are using some weird mail client, you can also check the
> mail header, making sure that mails from 1 to 5 have the field:
>
> In-Reply-To: <Message-Id of patch 0>
>
> Andi
Cheers,
Sam
Hi Sam,
On Thu, Mar 21, 2024 at 12:24:43AM -0600, Sam Edwards wrote:
> On 3/20/24 20:28, Andi Shyti wrote:
> > > Sorry about the resend; it seems my mail client "helpfully" swallowed the
> > > newlines on any line consisting only of whitespace, garbling the patches.
> >
> > I received three series from you:
> >
> > 1. [RESEND v2 RFC 1/5] i2c: mv64xxx: Clear bus errors before transfer
> > 2. [RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
> > 3. [RESEND RFC PATCH 0/5] Enhancements for mv64xxx I2C driver
> >
> > By the versioning, 1. is good, the rest is not good. Standing to
> > the time sent and comments in patch '0', 3. is good, the rest
> > not.
> >
> > Which one should be discarded? Can you please state it clearly?
..
> I sent the series in the order 2-3-1, so 1 is the version to look at (though
> I made no content changes between resends as I was only fighting my mail
> client's formatting).
so that it's the [RFC v2 ...] the right series... are you sure?
The order of arrival is:
1. Date: Tue, 19 Mar 2024 16:51:51 -0600
2. Date: Tue, 19 Mar 2024 19:40:51 -0600
3. Date: Tue, 19 Mar 2024 22:19:53 -0600
Anyway, I will take "1" as the good one, being a v2. I will
discard "2" and "3".
Then, please, do not forget next time the patch 0 and the
changelog.
..
> > Can you please make sure, next time (unless someone asks to
> > resend them again), that the patches are threaded? You can send
> > them to yourself first and see if they are really threaded.
>
> Yes, definitely. I take it from your phrasing that you're willing to collect
> the scattered mails yourself this one time only? If so, thank you for
> cleaning up after my mess. :)
>
> If not (and/or if someone else doesn't like the mess), I can always resend.
> I have already made one cleanup (removing the useless `default:` at the end
> of the FSM) so I guess it would technically be an "RFC v2" at this point.
For now no need to resend (unless someone complains). Let's give
it some time for review.
Andi
Hi Andi,
On 3/21/24 14:54, Andi Shyti wrote:
> so that it's the [RFC v2 ...] the right series... are you sure?
[RESEND v2 RFC ...] -- it's the second resend (thus third send), not the
second RFC (in retrospect I definitely should have used # instead of v)
>
> The order of arrival is:
>
> 1. Date: Tue, 19 Mar 2024 16:51:51 -0600
> 2. Date: Tue, 19 Mar 2024 19:40:51 -0600
> 3. Date: Tue, 19 Mar 2024 22:19:53 -0600
>
> Anyway, I will take "1" as the good one, being a v2. I will
> discard "2" and "3".
>
> Then, please, do not forget next time the patch 0 and the
> changelog.
Patch 0 was probably separated by the lack of threading but can be found
here: https://www.spinics.net/lists/linux-i2c/msg68235.html
There is no changelog as there were no changes to the patch content vs.
either of the first two sending attempts; I was only trying a different
way of navigating the minefield of mail agents that make whitespace
changes without my consent. :)
>
> ...
>
>>> Can you please make sure, next time (unless someone asks to
>>> resend them again), that the patches are threaded? You can send
>>> them to yourself first and see if they are really threaded.
>>
>> Yes, definitely. I take it from your phrasing that you're willing to collect
>> the scattered mails yourself this one time only? If so, thank you for
>> cleaning up after my mess. :)
>>
>> If not (and/or if someone else doesn't like the mess), I can always resend.
>> I have already made one cleanup (removing the useless `default:` at the end
>> of the FSM) so I guess it would technically be an "RFC v2" at this point.
>
> For now no need to resend (unless someone complains). Let's give
> it some time for review.
>
> Andi
Thanks again,
Sam