From: Simon Horman <simon.horman@netronome.com>
To: Po Liu <po.liu@nxp.com>
Cc: Claudiu Manoil <claudiu.manoil@nxp.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"vinicius.gomes@intel.com" <vinicius.gomes@intel.com>,
Vladimir Oltean <vladimir.oltean@nxp.com>,
Alexandru Marginean <alexandru.marginean@nxp.com>,
Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
Roy Zang <roy.zang@nxp.com>, Mingkai Hu <mingkai.hu@nxp.com>,
Jerry Huang <jerry.huang@nxp.com>, Leo Li <leoyang.li@nxp.com>
Subject: Re: [EXT] Re: [net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload
Date: Tue, 12 Nov 2019 14:50:45 +0100 [thread overview]
Message-ID: <20191112135045.5qaau7kqdxrrpqo4@netronome.com> (raw)
In-Reply-To: <VE1PR04MB6496CE5A0DA25D7AF9FD666492770@VE1PR04MB6496.eurprd04.prod.outlook.com>
On Tue, Nov 12, 2019 at 11:19:43AM +0000, Po Liu wrote:
...
> > > +/* class 5, command 0 */
> > > +struct tgs_gcl_conf {
> > > + u8 atc; /* init gate value */
> > > + u8 res[7];
> > > + union {
> > > + struct {
> > > + u8 res1[4];
> > > + __le16 acl_len;
> >
> > Given that u* types are used in this structure I think le16 would be more
> > appropriate than __le16.
>
> Here keep the same code style of this .h file. I think it is better to have another patch to fix them all. Do you agree?
>
> >
> > > + u8 res2[2];
> > > + };
> > > + struct {
> > > + u32 cctl;
> > > + u32 ccth;
> > > + };
> >
> > I'm a little surprised to see host endian values in a structure that appears to be
> > written to hardware. Is this intentional?
>
> Will remove.
If the HW defines these fields then I think its fine to leave them,
though with the correct byte-order.
I was more asking if it is intentional that the value for these
fields, when sent to the HW, is always zero in the context of this
patch-set. Likewise elsewhere.
...
> > > +
> > > + gcl_data->ct = cpu_to_le32(admin_conf->cycle_time);
> > > + gcl_data->cte = cpu_to_le32(admin_conf->cycle_time_extension);
> > > +
> > > + for (i = 0; i < gcl_len; i++) {
> > > + struct tc_taprio_sched_entry *temp_entry;
> > > + struct gce *temp_gce = gce + i;
> > > +
> > > + temp_entry = &admin_conf->entries[i];
> > > +
> > > + temp_gce->gate = cpu_to_le32(temp_entry->gate_mask);
> >
> > Gate is a u8 followed by 3 reserved bytes.
> > Perhaps there needs to be some bounds checking on
> > the value stored there given that the source is 32bits wide.
> >
> > Also, its not clear to me that the above logic, which I assume
> > takes the last significant byte of a 32bit value, works on
> > big endian systems as the 32bit value is always little endian.
>
> temp_entry->gate_mask is 32bit for wide possible input. Here change to hardware set 8bit wide.
> Can it just be like:
> temp_gce->gate = (u8) temp_entry->gate_mask;
I think that would be better.
Perhaps its best to also mask out the unwanted bits.
...
next prev parent reply other threads:[~2019-11-12 13:50 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-11 4:41 [net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload Po Liu
2019-11-11 4:41 ` [net-next, 2/2] enetc: update TSN Qbv PSPEED set according to adjust link speed Po Liu
2019-11-12 8:42 ` [v2,net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload Po Liu
2019-11-12 8:42 ` [v2,net-next, 2/2] enetc: update TSN Qbv PSPEED set according to adjust link speed Po Liu
2019-11-12 18:57 ` David Miller
2019-11-14 5:12 ` [v3,net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload Po Liu
2019-11-14 5:12 ` [v3,net-next, 2/2] enetc: update TSN Qbv PSPEED set according to adjust link speed Po Liu
2019-11-15 3:33 ` [v4,net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload Po Liu
2019-11-15 3:33 ` [v4,net-next, 2/2] enetc: update TSN Qbv PSPEED set according to adjust link speed Po Liu
2019-11-16 20:49 ` David Miller
2019-11-15 15:20 ` [v4,net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload Ivan Khoronzhuk
2019-11-16 20:49 ` David Miller
2019-11-14 18:51 ` [v3,net-next, " Ivan Khoronzhuk
2019-11-15 2:37 ` [EXT] " Po Liu
2019-11-12 18:57 ` [v2,net-next, " David Miller
2019-11-12 21:10 ` Ivan Khoronzhuk
2019-11-13 3:45 ` [EXT] " Po Liu
2019-11-13 13:41 ` Ivan Khoronzhuk
2019-11-14 4:39 ` Po Liu
2019-11-12 9:41 ` [net-next, 2/2] enetc: update TSN Qbv PSPEED set according to adjust link speed Simon Horman
2019-11-12 19:59 ` kbuild test robot
2019-11-12 19:59 ` kbuild test robot
2019-11-12 5:50 ` [net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload David Miller
2019-11-12 6:48 ` [EXT] " Po Liu
2019-11-12 9:41 ` Simon Horman
2019-11-12 11:19 ` [EXT] " Po Liu
2019-11-12 11:54 ` Claudiu Manoil
2019-11-12 13:46 ` Simon Horman
2019-11-12 13:50 ` Simon Horman [this message]
2019-11-12 18:58 ` David Miller
2019-11-12 18:59 ` David Miller
2019-11-13 4:55 ` [EXT] " Po Liu
2019-11-12 12:31 ` kbuild test robot
2019-11-12 12:31 ` kbuild test robot
2019-11-12 12:43 ` kbuild test robot
2019-11-12 12:43 ` kbuild test robot
2019-11-12 16:03 ` Ivan Khoronzhuk
2019-11-12 22:57 ` [RFC PATCH] enetc: enetc_setup_tc_mqprio() can be static kbuild test robot
2019-11-12 22:57 ` kbuild test robot
2019-11-12 22:57 ` [net-next, 1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload kbuild test robot
2019-11-12 22:57 ` kbuild test robot
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=20191112135045.5qaau7kqdxrrpqo4@netronome.com \
--to=simon.horman@netronome.com \
--cc=alexandru.marginean@nxp.com \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=jerry.huang@nxp.com \
--cc=leoyang.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingkai.hu@nxp.com \
--cc=netdev@vger.kernel.org \
--cc=po.liu@nxp.com \
--cc=roy.zang@nxp.com \
--cc=vinicius.gomes@intel.com \
--cc=vladimir.oltean@nxp.com \
--cc=xiaoliang.yang_1@nxp.com \
/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: link
Be 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.