| Message ID | 20221215193245.48314-2-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for 32 bit physical address | expand |
On 15.12.2022 20:32, Ayan Kumar Halder wrote: > As "io_size" and "uart->io_size" are both u64, so there will be no truncation. > Thus, one can remove the ASSERT() and extra assignment. > > In an earlier commit (7c1de0038895), > "ns16550.io_size" was u32 and "io_size" was u64. Thus, the ASSERT() was needed > to check if the values are the same. > However, in a later commit (c9f8e0aee507), > "ns16550.io_size" was changed to u64. Thus, the ASSERT() became redundant. > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> With the title improved to indicate what component is actually being touched Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
Hi, In the previous version, I have suggested the following title: xen/ns16550: Remove unneeded truncation check in the DT init code This would also address Jan's comment. On 15/12/2022 19:32, Ayan Kumar Halder wrote: > As "io_size" and "uart->io_size" are both u64, so there will be no truncation. > Thus, one can remove the ASSERT() and extra assignment. > > In an earlier commit (7c1de0038895), Why is this line shorter than the others? > "ns16550.io_size" was u32 and "io_size" was u64. Thus, the ASSERT() was needed > to check if the values are the same. > However, in a later commit (c9f8e0aee507), > "ns16550.io_size" was changed to u64. Thus, the ASSERT() became redundant. You missed my comment here: "Those two paragraphs are explaining why the truncation check is removed. So I think they should be moved first. Then you can add the initial paragraph to explain the resolution". > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > --- > > xen/drivers/char/ns16550.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index 01a05c9aa8..58d0ccd889 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -1747,7 +1747,6 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev, > struct ns16550 *uart; > int res; > u32 reg_shift, reg_width; > - u64 io_size; > > uart = &ns16550_com[0]; > > @@ -1758,14 +1757,10 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev, > uart->parity = UART_PARITY_NONE; > uart->stop_bits = 1; > > - res = dt_device_get_address(dev, 0, &uart->io_base, &io_size); > + res = dt_device_get_address(dev, 0, &uart->io_base, &uart->io_size); > if ( res ) > return res; > > - uart->io_size = io_size; > - > - ASSERT(uart->io_size == io_size); /* Detect truncation */ > - > res = dt_property_read_u32(dev, "reg-shift", ®_shift); > if ( !res ) > uart->reg_shift = 0; Cheers,
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index 01a05c9aa8..58d0ccd889 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1747,7 +1747,6 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev, struct ns16550 *uart; int res; u32 reg_shift, reg_width; - u64 io_size; uart = &ns16550_com[0]; @@ -1758,14 +1757,10 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev, uart->parity = UART_PARITY_NONE; uart->stop_bits = 1; - res = dt_device_get_address(dev, 0, &uart->io_base, &io_size); + res = dt_device_get_address(dev, 0, &uart->io_base, &uart->io_size); if ( res ) return res; - uart->io_size = io_size; - - ASSERT(uart->io_size == io_size); /* Detect truncation */ - res = dt_property_read_u32(dev, "reg-shift", ®_shift); if ( !res ) uart->reg_shift = 0;
As "io_size" and "uart->io_size" are both u64, so there will be no truncation. Thus, one can remove the ASSERT() and extra assignment. In an earlier commit (7c1de0038895), "ns16550.io_size" was u32 and "io_size" was u64. Thus, the ASSERT() was needed to check if the values are the same. However, in a later commit (c9f8e0aee507), "ns16550.io_size" was changed to u64. Thus, the ASSERT() became redundant. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- xen/drivers/char/ns16550.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)