> +struct oa_tc6 {
> + struct spi_device *spi;
> + bool ctrl_prot;
> +};
Should this be considered an opaque structure which the MAC driver
should not access the members?
I don't see anything setting ctrl_prot here. Does it need a setter and
a getter?
Andrew
Hi Andrew,
On 13/09/23 7:46 am, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
>> +struct oa_tc6 {
>> + struct spi_device *spi;
>> + bool ctrl_prot;
>> +};
>
> Should this be considered an opaque structure which the MAC driver
> should not access the members?
>
> I don't see anything setting ctrl_prot here. Does it need a setter and
> a getter?
Ah ok, it is supposed to be done in the oa_tc6_init() function. Somehow
missed it. Will correct it in the next version.
Best Regards,
Parthiban V
>
> Andrew
On Tue, Sep 19, 2023 at 11:13:13AM +0000, [email protected] wrote:
> Hi Andrew,
>
> On 13/09/23 7:46 am, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> >> +struct oa_tc6 {
> >> + struct spi_device *spi;
> >> + bool ctrl_prot;
> >> +};
> >
> > Should this be considered an opaque structure which the MAC driver
> > should not access the members?
Opaque vs not opaque is an important design decision. If the MAC
driver is allowed to directly access this structure, you should
document the locking concept. If the MAC is not supposed to access it
directly, only uses getters/setters, that also needs documenting, and
maybe even make it a void * in the MAC driver.
Andrew
Hi Andrew,
On 19/09/23 6:28 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Tue, Sep 19, 2023 at 11:13:13AM +0000, [email protected] wrote:
>> Hi Andrew,
>>
>> On 13/09/23 7:46 am, Andrew Lunn wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>>> +struct oa_tc6 {
>>>> + struct spi_device *spi;
>>>> + bool ctrl_prot;
>>>> +};
>>>
>>> Should this be considered an opaque structure which the MAC driver
>>> should not access the members?
>
> Opaque vs not opaque is an important design decision. If the MAC
> driver is allowed to directly access this structure, you should
> document the locking concept. If the MAC is not supposed to access it
> directly, only uses getters/setters, that also needs documenting, and
> maybe even make it a void * in the MAC driver.
Sorry that I missed to reply in the previous email. Thanks for the
details on this topic.
Yes, as "struct oa_tc6" and its members are not or going to be accessed
in the MAC driver, I will consider this as an opaque structure and
declare it as void *opaque_oa_tc6 in the MAC driver private structure
"struct lan865x_priv" and will pass to the APIs exported from oa_tc6.c lib.
Is my understanding correct?
Best Regards,
Parthiban V
>
> Andrew
> Yes, as "struct oa_tc6" and its members are not or going to be accessed
> in the MAC driver, I will consider this as an opaque structure and
> declare it as void *opaque_oa_tc6 in the MAC driver private structure
> "struct lan865x_priv" and will pass to the APIs exported from oa_tc6.c lib.
>
> Is my understanding correct?
Yes.
If the structure is going to be truly opaque, i suggest having it in
the C file, not the H file.
Andrew
Hi Andrew,
On 22/09/23 12:49 am, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
>> Yes, as "struct oa_tc6" and its members are not or going to be accessed
>> in the MAC driver, I will consider this as an opaque structure and
>> declare it as void *opaque_oa_tc6 in the MAC driver private structure
>> "struct lan865x_priv" and will pass to the APIs exported from oa_tc6.c lib.
>>
>> Is my understanding correct?
>
> Yes.
>
> If the structure is going to be truly opaque, i suggest having it in
> the C file, not the H file.
Yes for sure, I will move it to oa_tc6.c file.
Andrew, thanks a lot for all your comments. They are all really helping
me to know/learn many things to improve my patches and coding style.
Please keep supporting.
Best Regards,
Parthiban V
>
> Andrew
Hi Andrew,
As I didn't receive any more comments for past couple days, shall I
start preparing v2 patch series with comments already given?
Best Regards,
Parthiban V
-------- Forwarded Message --------
Subject: Re: [RFC PATCH net-next 1/6] net: ethernet: implement OPEN
Alliance control transaction interface
Date: Fri, 22 Sep 2023 10:09:09 +0530
From: Parthiban Veerasooran <[email protected]>
To: Andrew Lunn <[email protected]>
CC: [email protected], [email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected], [email protected],
[email protected], [email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected]
Hi Andrew,
On 22/09/23 12:49 am, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
>> Yes, as "struct oa_tc6" and its members are not or going to be accessed
>> in the MAC driver, I will consider this as an opaque structure and
>> declare it as void *opaque_oa_tc6 in the MAC driver private structure
>> "struct lan865x_priv" and will pass to the APIs exported from oa_tc6.c lib.
>>
>> Is my understanding correct?
>
> Yes.
>
> If the structure is going to be truly opaque, i suggest having it in
> the C file, not the H file.
Yes for sure, I will move it to oa_tc6.c file.
Andrew, thanks a lot for all your comments. They are all really helping
me to know/learn many things to improve my patches and coding style.
Please keep supporting.
Best Regards,
Parthiban V
>
> Andrew