RE: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
Hi Michal, > -Original Message- > Subject: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments > > In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment > stating that the guest is expected to read the DR register only if the > TXFE bit of FR register is not set. This is obviously logically wrong and > it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read). > > Signed-off-by: Michal Orzel Reviewed-by: Henry Wang Kind regards, Henry
Re: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
On Thu, 6 Apr 2023, Ayan Kumar Halder wrote:
> On 05/04/2023 12:17, Michal Orzel wrote:
> > In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
> > stating that the guest is expected to read the DR register only if the
> > TXFE bit of FR register is not set. This is obviously logically wrong and
> > it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
> NIT:- I will prefer if the PL011 TRM is mentioned with the relevant section.
> >
> > Signed-off-by: Michal Orzel
> Reviewed-by: Ayan Kumar Halder
Reviewed-by: Stefano Stabellini
> > ---
> > xen/arch/arm/vpl011.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index 2fa80bc15ac4..0186d8a31834 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -143,8 +143,8 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
> > /*
> >* It is expected that there will be data in the ring buffer when this
> >* function is called since the guest is expected to read the data
> > register
> > - * only if the TXFE flag is not set.
> > - * If the guest still does read when TXFE bit is set then 0 will be
> > returned.
> > + * only if the RXFE flag is not set.
> > + * If the guest still does read when RXFE bit is set then 0 will be
> > returned.
> >*/
> > if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
> > {
> > @@ -202,8 +202,8 @@ static uint8_t vpl011_read_data(struct domain *d)
> > /*
> >* It is expected that there will be data in the ring buffer when this
> >* function is called since the guest is expected to read the data
> > register
> > - * only if the TXFE flag is not set.
> > - * If the guest still does read when TXFE bit is set then 0 will be
> > returned.
> > + * only if the RXFE flag is not set.
> > + * If the guest still does read when RXFE bit is set then 0 will be
> > returned.
> >*/
> > if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
> > {
>
Re: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
On 05/04/2023 12:17, Michal Orzel wrote:
In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
stating that the guest is expected to read the DR register only if the
TXFE bit of FR register is not set. This is obviously logically wrong and
it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
NIT:- I will prefer if the PL011 TRM is mentioned with the relevant section.
Signed-off-by: Michal Orzel
Reviewed-by: Ayan Kumar Halder
---
xen/arch/arm/vpl011.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 2fa80bc15ac4..0186d8a31834 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -143,8 +143,8 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
/*
* It is expected that there will be data in the ring buffer when this
* function is called since the guest is expected to read the data
register
- * only if the TXFE flag is not set.
- * If the guest still does read when TXFE bit is set then 0 will be
returned.
+ * only if the RXFE flag is not set.
+ * If the guest still does read when RXFE bit is set then 0 will be
returned.
*/
if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
{
@@ -202,8 +202,8 @@ static uint8_t vpl011_read_data(struct domain *d)
/*
* It is expected that there will be data in the ring buffer when this
* function is called since the guest is expected to read the data
register
- * only if the TXFE flag is not set.
- * If the guest still does read when TXFE bit is set then 0 will be
returned.
+ * only if the RXFE flag is not set.
+ * If the guest still does read when RXFE bit is set then 0 will be
returned.
*/
if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
{
[PATCH 1/3] xen/arm: vpl011: Fix misleading comments
In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
stating that the guest is expected to read the DR register only if the
TXFE bit of FR register is not set. This is obviously logically wrong and
it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
Signed-off-by: Michal Orzel
---
xen/arch/arm/vpl011.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 2fa80bc15ac4..0186d8a31834 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -143,8 +143,8 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
/*
* It is expected that there will be data in the ring buffer when this
* function is called since the guest is expected to read the data register
- * only if the TXFE flag is not set.
- * If the guest still does read when TXFE bit is set then 0 will be
returned.
+ * only if the RXFE flag is not set.
+ * If the guest still does read when RXFE bit is set then 0 will be
returned.
*/
if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
{
@@ -202,8 +202,8 @@ static uint8_t vpl011_read_data(struct domain *d)
/*
* It is expected that there will be data in the ring buffer when this
* function is called since the guest is expected to read the data register
- * only if the TXFE flag is not set.
- * If the guest still does read when TXFE bit is set then 0 will be
returned.
+ * only if the RXFE flag is not set.
+ * If the guest still does read when RXFE bit is set then 0 will be
returned.
*/
if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
{
--
2.25.1
