Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add switch api for clause 22 mdio access #1507

Merged
merged 5 commits into from
Oct 10, 2022

Conversation

jiahua-wang
Copy link
Contributor

@jiahua-wang jiahua-wang commented Jun 23, 2022

Signed-off-by: Jiahua Wang jiahua.wang@broadcom.com

    Why I did it
    There are 2 mdio access modes: clause 45 and clause 22. The existing sai
    switch api "switch_mdio_read/switch_mdio_write" does not distinguish the
    2 modes.

    How I did it
    The new sai switch api "switch_mdio_cl22_ead/switch_mdio_cl22_write" are
    added for clause 22 mdio access only.

    How to verify it
    On a platform with mdio clause 22 device connected to NPU mdio bus,
    PAI/gearbox will only work with the new new sai switch api when there is
    no other means to configure the mdio access mode.

Signed-off-by: Jiahua Wang <jiahua.wang@broadcom.com>
inc/saiswitch.h Outdated
@@ -3272,6 +3272,8 @@ typedef struct _sai_switch_api_t
sai_remove_switch_tunnel_fn remove_switch_tunnel;
sai_set_switch_tunnel_attribute_fn set_switch_tunnel_attribute;
sai_get_switch_tunnel_attribute_fn get_switch_tunnel_attribute;
sai_switch_mdio_read_fn switch_mdio_cl22_read;
sai_switch_mdio_write_fn switch_mdio_cl22_write;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add comments on what is the difference bewteen the switch_mdio_read and switch_mdio_cl22_read? why need to add these new api?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MDIO access can be 2 types, clause 45 and clause 22. While most MDIO devices use clause 45, there are still some 1G or lower speed PHY devices use clause 22. The SAI switch api functions switch_mdio_read/switch_mdio_write are used for clause 45 mdio device access. The newly added SAI switch api functions switch_mdio_cl22_read/switch_mdio_cl22_write are used for clause 22 mdio device access.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add this to your commit message so that we can merge with proper commit message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed the commit message to:
Author: Jiahua Wang jiahua.wang@broadcom.com
Date: Mon Jul 25 10:31:10 2022 -0700

Merge branch 'switch_mdio_cl22' of https://github.com/jiahua-wang/SAI into switch_mdio_cl22

commit c47ee84
Author: Jiahua Wang jiahua.wang@broadcom.com
Date: Tue Jul 12 12:16:58 2022 -0700

Add sai_switch_mdio_cl22_read_fn and sai_switch_mdio_cl22_write_fn

Signed-off-by: Jiahua Wang <jiahua.wang@broadcom.com>

    Why I did it
    There are 2 mdio access modes: clause 45 and clause 22. The existing sai
    switch api "switch_mdio_read/switch_mdio_write" does not distinguish the
    2 modes.

    How I did it
    The new sai switch api "switch_mdio_cl22_ead/switch_mdio_cl22_write" are
    added for clause 22 mdio access only.

    How to verify it
    On a platform with mdio clause 22 device connected to NPU mdio bus,
    PAI/gearbox will only work with the new new sai switch api when there is
    no other means to configure the mdio access mode.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the expectation is that switch_mdio_read/write will be used for clause 45 only, and switch_mdio_cl22_read/write will be used for clause 22?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switch_mdio_read/write will be used for clause 45 only with regular usage. But it can be used for clause 22 under other configuration/control, e.g. soc property/experimental attribute.

@lguohan
Copy link
Collaborator

lguohan commented Jun 23, 2022

also, can you check the build failure?

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 24, 2022

you will also need to refactor parse.pl to pass sanitycheck

Signed-off-by: Jiahua Wang <jiahua.wang@broadcom.com>
Signed-off-by: Jiahua Wang <jiahua.wang@broadcom.com>
Signed-off-by: Jiahua Wang <jiahua.wang@broadcom.com>

    Why I did it
    There are 2 mdio access modes: clause 45 and clause 22. The existing sai
    switch api "switch_mdio_read/switch_mdio_write" does not distinguish the
    2 modes.

    How I did it
    The new sai switch api "switch_mdio_cl22_ead/switch_mdio_cl22_write" are
    added for clause 22 mdio access only.

    How to verify it
    On a platform with mdio clause 22 device connected to NPU mdio bus,
    PAI/gearbox will only work with the new new sai switch api when there is
    no other means to configure the mdio access mode.
* @param[in] number_of_registers Number of consecutive registers to read
* @param[out] reg_val Register read values
*/
typedef sai_status_t (*sai_switch_mdio_cl22_read_fn)(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type sai_switch_mdio_cl22_read_fn is same as sai_switch_mdio_read_fn. It looks not needed to define it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the first commit: 912c4c4 The new cl22 functions had the type you suggested. But it would not pass the parse.pl sanity check. I changed the cl22 function to the new type in the last commit: 0fb1c06

@@ -3272,6 +3312,8 @@ typedef struct _sai_switch_api_t
sai_remove_switch_tunnel_fn remove_switch_tunnel;
sai_set_switch_tunnel_attribute_fn set_switch_tunnel_attribute;
sai_get_switch_tunnel_attribute_fn get_switch_tunnel_attribute;
sai_switch_mdio_cl22_read_fn switch_mdio_cl22_read;
sai_switch_mdio_cl22_write_fn switch_mdio_cl22_write;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be:

sai_switch_mdio_read_fn          switch_mdio_cl22_read
sai_switch_mdio_write_fn         switch_mdio_cl22_write

Copy link
Contributor Author

@jiahua-wang jiahua-wang Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the first commit: 912c4c4 The new cl22 functions had the type you suggested. But it would not pass the parse.pl sanity check. I changed the cl22 function to the new type in the last commit: 0fb1c06

@lguohan lguohan merged commit 1b268a8 into opencomputeproject:master Oct 10, 2022
@rlhui rlhui mentioned this pull request Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants