| Message ID | 1526561446-29004-4-git-send-email-patrice.chotard@st.com |
|---|---|
| State | Accepted |
| Commit | eae4764f1ac2f3968a5c87a17b2e894a5fe05d0b |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Update STM32 serial driver | expand |
Hi Patrick, On 17 May 2018 at 06:50, Patrice Chotard <patrice.chotard@st.com> wrote: > From: Patrick Delaunay <patrick.delaunay@st.com> > > Implements serial setparity ops to allow uart parity change. > It allows to select ODD, EVEN or NONE parity. > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > Signed-off-by: Patrice Chotard <patrice.chotard@st.com> > --- > > include/serial.h | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/include/serial.h b/include/serial.h > index 384df94ed0b3..b9ef6d91c9c5 100644 > --- a/include/serial.h > +++ b/include/serial.h > @@ -67,6 +67,12 @@ extern int usbtty_tstc(void); > > struct udevice; > > +enum serial_par { > + SERIAL_PAR_NONE, > + SERIAL_PAR_ODD, > + SERIAL_PAR_EVEN > +}; > + > /** > * struct struct dm_serial_ops - Driver model serial operations > * > @@ -143,6 +149,16 @@ struct dm_serial_ops { > */ > int (*loop)(struct udevice *dev, int on); > #endif > + /** > + * setparity() - Set up the parity > + * > + * Set up a new parity for this device. > + * > + * @dev: Device pointer > + * @parity: parity to use > + * @return 0 if OK, -ve on error > + */ > + int (*setparity)(struct udevice *dev, enum serial_par parity); To me it seems that changing parity while in the middle of operation might be tricky. I suppose this follows along with setbrg() so fair enough. But I worry about adding more operations here. The next thing to come is presumably the length (7 bits, 8 bits, ...). Perhaps we should have a more generic setconfig() which change change speed, format and parity all at once? For format and parity, we could have a 'default' parameter value. Also, there should be a corresponding function in serial-uclass.c and ideal a call from some sandbox test (although I see at present we don't have test/dm/serial.c) Regards, Simon
Hi Simon On 05/18/2018 03:37 AM, Simon Glass wrote: > Hi Patrick, > > On 17 May 2018 at 06:50, Patrice Chotard <patrice.chotard@st.com> wrote: >> From: Patrick Delaunay <patrick.delaunay@st.com> >> >> Implements serial setparity ops to allow uart parity change. >> It allows to select ODD, EVEN or NONE parity. >> >> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> >> Signed-off-by: Patrice Chotard <patrice.chotard@st.com> >> --- >> >> include/serial.h | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/include/serial.h b/include/serial.h >> index 384df94ed0b3..b9ef6d91c9c5 100644 >> --- a/include/serial.h >> +++ b/include/serial.h >> @@ -67,6 +67,12 @@ extern int usbtty_tstc(void); >> >> struct udevice; >> >> +enum serial_par { >> + SERIAL_PAR_NONE, >> + SERIAL_PAR_ODD, >> + SERIAL_PAR_EVEN >> +}; >> + >> /** >> * struct struct dm_serial_ops - Driver model serial operations >> * >> @@ -143,6 +149,16 @@ struct dm_serial_ops { >> */ >> int (*loop)(struct udevice *dev, int on); >> #endif >> + /** >> + * setparity() - Set up the parity >> + * >> + * Set up a new parity for this device. >> + * >> + * @dev: Device pointer >> + * @parity: parity to use >> + * @return 0 if OK, -ve on error >> + */ >> + int (*setparity)(struct udevice *dev, enum serial_par parity); > > To me it seems that changing parity while in the middle of operation > might be tricky. I suppose this follows along with setbrg() so fair > enough. > > But I worry about adding more operations here. The next thing to come > is presumably the length (7 bits, 8 bits, ...). Perhaps we should have > a more generic setconfig() which change change speed, format and > parity all at once? For format and parity, we could have a 'default' > parameter value. Ok that make sense, i will resubmit a v2 > > Also, there should be a corresponding function in serial-uclass.c and > ideal a call from some sandbox test (although I see at present we > don't have test/dm/serial.c) Ok Patrice > > Regards, > Simon >
On Thu, May 17, 2018 at 02:50:44PM +0200, Patrice Chotard wrote: > From: Patrick Delaunay <patrick.delaunay@st.com> > > Implements serial setparity ops to allow uart parity change. > It allows to select ODD, EVEN or NONE parity. > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > Signed-off-by: Patrice Chotard <patrice.chotard@st.com> Applied to u-boot/master, thanks!
Hi Tom On 05/28/2018 09:13 PM, Tom Rini wrote: > On Thu, May 17, 2018 at 02:50:44PM +0200, Patrice Chotard wrote: > >> From: Patrick Delaunay <patrick.delaunay@st.com> >> >> Implements serial setparity ops to allow uart parity change. >> It allows to select ODD, EVEN or NONE parity. >> >> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> >> Signed-off-by: Patrice Chotard <patrice.chotard@st.com> > > Applied to u-boot/master, thanks! > After Simon's review, i expected to send a v2 of this series but you was too fast ;-). 2 solutions: _ i submit a new patchset including Simon's remarks on top of current master branch _ you revert this series and i submit the v2 What do you prefer ? Thanks Patrice
On Tue, May 29, 2018 at 07:13:36AM +0000, Patrice CHOTARD wrote: > Hi Tom > > On 05/28/2018 09:13 PM, Tom Rini wrote: > > On Thu, May 17, 2018 at 02:50:44PM +0200, Patrice Chotard wrote: > > > >> From: Patrick Delaunay <patrick.delaunay@st.com> > >> > >> Implements serial setparity ops to allow uart parity change. > >> It allows to select ODD, EVEN or NONE parity. > >> > >> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > >> Signed-off-by: Patrice Chotard <patrice.chotard@st.com> > > > > Applied to u-boot/master, thanks! > > > > After Simon's review, i expected to send a v2 of this series but you was > too fast ;-). > > 2 solutions: > _ i submit a new patchset including Simon's remarks on top of current > master branch > _ you revert this series and i submit the v2 > > What do you prefer ? Ugh, oops. I swear it was good when I checked, but maybe I was too quick. A follow-up series please, thanks!
diff --git a/include/serial.h b/include/serial.h index 384df94ed0b3..b9ef6d91c9c5 100644 --- a/include/serial.h +++ b/include/serial.h @@ -67,6 +67,12 @@ extern int usbtty_tstc(void); struct udevice; +enum serial_par { + SERIAL_PAR_NONE, + SERIAL_PAR_ODD, + SERIAL_PAR_EVEN +}; + /** * struct struct dm_serial_ops - Driver model serial operations * @@ -143,6 +149,16 @@ struct dm_serial_ops { */ int (*loop)(struct udevice *dev, int on); #endif + /** + * setparity() - Set up the parity + * + * Set up a new parity for this device. + * + * @dev: Device pointer + * @parity: parity to use + * @return 0 if OK, -ve on error + */ + int (*setparity)(struct udevice *dev, enum serial_par parity); }; /**