| Message ID | 20180828110921.2542-5-david.gounaris@infinera.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Ethernet over hdlc | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
From: David Gounaris <david.gounaris@infinera.com> Date: 2018/8/28 19:09 > Subject: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value > > Set default HMASK to 0x0000 to use > promiscuous mode in the hdlc controller. > > Signed-off-by: David Gounaris <david.gounaris@infinera.com> > --- > drivers/net/wan/fsl_ucc_hdlc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h > index c21134c1f180..5bc3d1a6ca6e 100644 > --- a/drivers/net/wan/fsl_ucc_hdlc.h > +++ b/drivers/net/wan/fsl_ucc_hdlc.h > @@ -134,7 +134,7 @@ struct ucc_hdlc_private { > > #define HDLC_HEAD_MASK 0x0000 > #define DEFAULT_HDLC_HEAD 0xff44 > -#define DEFAULT_ADDR_MASK 0x00ff > +#define DEFAULT_ADDR_MASK 0x0000 > #define DEFAULT_HDLC_ADDR 0x00ff > > #define BMR_GBL 0x20000000 > -- It is not proper to set default HMASK to 0x0000, how about to add a new property standing for hmask into device tree, If get this property from dtb, then set it with the value from dtb, otherwise, set it with default HMASK ox00ff? Best Regards Qiang Zhao
Le 29/08/2018 à 04:54, Qiang Zhao a écrit : > From: David Gounaris <david.gounaris@infinera.com> > Date: 2018/8/28 19:09 >> Subject: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value >> >> Set default HMASK to 0x0000 to use >> promiscuous mode in the hdlc controller. Why do you need to do that ? An HDLC frame encapsulating Ethernet should look like: HDLC Frame includes: – Opening Flag 7E hex – Address field FF hex – Control field 03 hex – Information field Original Ethernet Packet, 1522 octets max. – FCS CRC16 (2 bytes) – Closing Flag 7E hex What do you mean by 'promiscuous mode' ? In any case, should a filter mask be changed for promiscuous mode, I believe the change should be done at the time you enter promiscuous mode, not at all time. Christophe >> >> Signed-off-by: David Gounaris <david.gounaris@infinera.com> >> --- >> drivers/net/wan/fsl_ucc_hdlc.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h >> index c21134c1f180..5bc3d1a6ca6e 100644 >> --- a/drivers/net/wan/fsl_ucc_hdlc.h >> +++ b/drivers/net/wan/fsl_ucc_hdlc.h >> @@ -134,7 +134,7 @@ struct ucc_hdlc_private { >> >> #define HDLC_HEAD_MASK 0x0000 >> #define DEFAULT_HDLC_HEAD 0xff44 >> -#define DEFAULT_ADDR_MASK 0x00ff >> +#define DEFAULT_ADDR_MASK 0x0000 >> #define DEFAULT_HDLC_ADDR 0x00ff >> >> #define BMR_GBL 0x20000000 >> -- > > It is not proper to set default HMASK to 0x0000, how about to add a new property standing for hmask into device tree, > If get this property from dtb, then set it with the value from dtb, otherwise, set it with default HMASK ox00ff? > > Best Regards > Qiang Zhao >
On Wed, 2018-08-29 at 09:18 +0200, Christophe LEROY wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > Le 29/08/2018 à 04:54, Qiang Zhao a écrit : > > From: David Gounaris <david.gounaris@infinera.com> > > Date: 2018/8/28 19:09 > > > Subject: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value > > > > > > Set default HMASK to 0x0000 to use > > > promiscuous mode in the hdlc controller. > > Why do you need to do that ? > > An HDLC frame encapsulating Ethernet should look like: > > HDLC Frame includes: > – Opening Flag 7E hex > – Address field FF hex > – Control field 03 hex > – Information field Original Ethernet Packet, 1522 octets max. > – FCS CRC16 (2 bytes) > – Closing Flag 7E hex In our case the Eth frame starts directly after the Opening Flag so there is no FF address field. > > What do you mean by 'promiscuous mode' ? Just that we don't want any address filtering of packets. I don't think this would be a problem for pure HDLC frames either, do you? > > In any case, should a filter mask be changed for promiscuous mode, I > believe the change should be done at the time you enter promiscuous > mode, not at all time. > > Christophe > > > > > > > Signed-off-by: David Gounaris <david.gounaris@infinera.com> > > > --- > > > drivers/net/wan/fsl_ucc_hdlc.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h > > > index c21134c1f180..5bc3d1a6ca6e 100644 > > > --- a/drivers/net/wan/fsl_ucc_hdlc.h > > > +++ b/drivers/net/wan/fsl_ucc_hdlc.h > > > @@ -134,7 +134,7 @@ struct ucc_hdlc_private { > > > > > > #define HDLC_HEAD_MASK 0x0000 > > > #define DEFAULT_HDLC_HEAD 0xff44 > > > -#define DEFAULT_ADDR_MASK 0x00ff > > > +#define DEFAULT_ADDR_MASK 0x0000 > > > #define DEFAULT_HDLC_ADDR 0x00ff > > > > > > #define BMR_GBL 0x20000000 > > > -- > > > > It is not proper to set default HMASK to 0x0000, how about to add a new property standing for hmask into device tree, > > If get this property from dtb, then set it with the value from dtb, otherwise, set it with default HMASK ox00ff? > > > > Best Regards > > Qiang Zhao > >
diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h index c21134c1f180..5bc3d1a6ca6e 100644 --- a/drivers/net/wan/fsl_ucc_hdlc.h +++ b/drivers/net/wan/fsl_ucc_hdlc.h @@ -134,7 +134,7 @@ struct ucc_hdlc_private { #define HDLC_HEAD_MASK 0x0000 #define DEFAULT_HDLC_HEAD 0xff44 -#define DEFAULT_ADDR_MASK 0x00ff +#define DEFAULT_ADDR_MASK 0x0000 #define DEFAULT_HDLC_ADDR 0x00ff #define BMR_GBL 0x20000000
Set default HMASK to 0x0000 to use promiscuous mode in the hdlc controller. Signed-off-by: David Gounaris <david.gounaris@infinera.com> --- drivers/net/wan/fsl_ucc_hdlc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)