All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sherry Sun <sherry.sun@nxp.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"jirislaby@kernel.org" <jirislaby@kernel.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>
Subject: RE: [PATCH] tty: serial: imx: initialize peripheral_config/peripheral_size for sdma config
Date: Wed, 3 Aug 2022 09:51:00 +0000	[thread overview]
Message-ID: <AS8PR04MB8404DA3B296728BD75D8D6BF929C9@AS8PR04MB8404.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20220803094041.cmjublrg3nu4odlt@pengutronix.de>


> Subject: Re: [PATCH] tty: serial: imx: initialize
> peripheral_config/peripheral_size for sdma config
> 
> On Wed, Aug 03, 2022 at 09:15:58AM +0000, Sherry Sun wrote:
> > > On 03.08.22 08:57, Sherry Sun wrote:
> > > > Since commit 824a0a02cd74 ("dmaengine: imx-sdma: Add multi fifo
> > > > support") adds the use of
> > > > dma_slave_config->peripheral_config/peripheral_size to sdma
> > > > driver, the client drivers like uart need to initialize the
> > > > peripheral_config/peripheral_size for sdma, otherwise, the random
> > > > value of local variable slave_config may cause unexpected
> > > peripheral_config and make sdma mess up.
> > > >
> > >
> > > If this a fix, please add a Fixes: tag. I am not sure it is though, see below.
> >
> > Hi Ahmad, thanks for the comments.
> > I don't think this patch is a fix for a specific commit, so we don't need the
> Fixes tag.
> >
> > >
> > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > > ---
> > > >  drivers/tty/serial/imx.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > > index
> > > > 522445a8f666..bb8c2a712e94 100644
> > > > --- a/drivers/tty/serial/imx.c
> > > > +++ b/drivers/tty/serial/imx.c
> > > > @@ -1320,6 +1320,8 @@ static int imx_uart_dma_init(struct imx_port
> > > > *sport)
> > >
> > > This function starts with
> > >
> > > struct dma_slave_config slave_config = {};
> > >
> > > >  	slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> > > >  	/* one byte less than the watermark level to enable the aging
> > > > timer
> > > */
> > > >  	slave_config.src_maxburst = RXTL_DMA - 1;
> > > > +	slave_config.peripheral_config = NULL;
> > > > +	slave_config.peripheral_size = 0;
> > >
> > > So these are already zero-initialized.
> >
> > I am not sure actually, I think initialize a struct with {} cannot
> > guarantee that all members are initialized to 0, it may depend on the
> > compiler.
> 
> I'm sure and even reverified in my C book[1].
> 
> 	struct mystruct variable = {};
> 
> results in all components being initialized to their default initial value (which
> is 0 for numbers and NULL for pointers). This works for automatic variables
> since "Standard C" which should cover everthing that is capable to compile
> today's kernel.
> 
> Best regards
> Uwe
> 
> [1] C - A Reference Manual, 978-0130895929
> 

Hi Uwe, wow, this really resolved my confusion, thanks a lot for your confirmation! ^_^

Best regards
Sherry

      reply	other threads:[~2022-08-03  9:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-03  6:57 [PATCH] tty: serial: imx: initialize peripheral_config/peripheral_size for sdma config Sherry Sun
2022-08-03  8:16 ` Ahmad Fatoum
2022-08-03  9:15   ` Sherry Sun
2022-08-03  9:22     ` gregkh
2022-08-03  9:30       ` Sherry Sun
2022-08-03  9:40     ` Uwe Kleine-König
2022-08-03  9:51       ` Sherry Sun [this message]

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=AS8PR04MB8404DA3B296728BD75D8D6BF929C9@AS8PR04MB8404.eurprd04.prod.outlook.com \
    --to=sherry.sun@nxp.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    /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.