Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 10 Dec 2020 11:00:06 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     =?iso-8859-1?Q?J=F3zsef_Horv=E1th?= <info@ministro.hu>
Cc:     =?iso-8859-1?B?Suly9G1l?= Pouiller <jerome.pouiller@silabs.com>,
        devel@driverdev.osuosl.org, devicetree@vger.kernel.org,
        "'Greg Kroah-Hartman'" <gregkh@linuxfoundation.org>,
        driverdev-devel@linuxdriverproject.org,
        linux-kernel@vger.kernel.org, "'Rob Herring'" <robh+dt@kernel.org>
Subject: Re: [PATCH] Staging: silabs si4455 serial driver
Message-ID: <20201210080006.GK2767@kadam>
References: <!&!AAAAAAAAAAAuAAAAAAAAAM7AkQxKEJRHh2BgMNSTrQkBAExvbAW64DNBoXXP8CRioZMAAAAzfOEAABAAAAAJUqiRO33GQqGIHffCVyG/AQAAAAA=@ministro.hu>
 <2907305.Mh6RI2rZIc@pc-42>
 <20201209194153.GC30918@dincontrollerdev>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20201209194153.GC30918@dincontrollerdev>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Wed, Dec 09, 2020 at 07:41:53PM +0000, J?zsef Horv?th wrote:
> > > +static int si4455_get_part_info(struct uart_port *port,
> > > +                               struct si4455_part_info *result)
> > > +{
> > > +       int ret;
> > > +       u8 dataOut[] = { SI4455_CMD_ID_PART_INFO };
> > > +       u8 dataIn[SI4455_CMD_REPLY_COUNT_PART_INFO];
> > > +
> > > +       ret = si4455_send_command_get_response(port,
> > > +                                               sizeof(dataOut),
> > > +                                               dataOut,
> > > +                                               sizeof(dataIn),
> > > +                                               dataIn);
> > 
> > Why not:
> > 
> 
> I changed all like this in my code already. I test it, and I'll send it again.
> 
> Ps.: For my eyes is better to read line or list, reading table is harder :)
> 
> 	line(arg1, arg2, arg3, arg4);
> 
> 	list(arg1,
> 		arg2,
> 		arg3,
> 		arg4);
> 
> 	table(arg1, arg2,
> 		arg3, arg4);
> 

Use spaces to make arguments have to line up properly.
`checkpatch.pl --strict` will complain if it's not done.

	table(arg1, arg2,
	      arg_whatver, foo);
[tab][space x 7]arg_whaver, foo);

But I think J?r?me's main point was to get rid of the dataOut buffer and
use "result" directly.

> 
> >        ret = si4455_send_command_get_response(port,
> >                                               sizeof(*result), result,
> >                                               sizeof(dataIn), dataIn);
> > 
> > > +       if (ret == 0) {
> > > +               result->CHIPREV = dataIn[0];
> > > +               memcpy(&result->PART, &dataIn[1],sizeof(result->PART));
> > > +               result->PBUILD = dataIn[3];
> > > +               memcpy(&result->ID, &dataIn[4], sizeof(result->ID));
> > > +               result->CUSTOMER = dataIn[6];
> > > +               result->ROMID = dataIn[7];
> > > +               result->BOND = dataIn[8];
> > 
> > ... it would avoid all these lines.
> > 

regards,
dan carpenter