Hi Dan, Thanks a lot for your comments. Please see my comments inline. Best regards, Yangbo Lu > -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > Sent: Thursday, April 19, 2018 6:40 PM > To: Y.b. Lu <yangbo.lu@xxxxxxx> > Cc: devel@xxxxxxxxxxxxxxxxxxxx; Greg Kroah-Hartman > <gregkh@xxxxxxxxxxxxxxxxxxx>; Richard Cochran <richardcochran@xxxxxxxxx> > Subject: Re: [PATCH 1/2] staging: fsl-dpaa2/rtc: add rtc driver > > On Thu, Apr 19, 2018 at 04:31:36PM +0800, Yangbo Lu wrote: > > +int dprtc_open(struct fsl_mc_io *mc_io, > > + u32 cmd_flags, > > + int dprtc_id, > > + u16 *token) > > +{ > > + struct dprtc_cmd_open *cmd_params; > > + struct fsl_mc_command cmd = { 0 }; > > + int err; > > + > > + /* prepare command */ > > These comments are a bit obvious. Just remove them. [Y.b. Lu] Will remove them. > > > + cmd.header = mc_encode_cmd_header(DPRTC_CMDID_OPEN, > > + cmd_flags, > > + 0); > > + cmd_params = (struct dprtc_cmd_open *)cmd.params; > > + cmd_params->dprtc_id = cpu_to_le32(dprtc_id); > > + > > + /* send command to mc*/ > > remove [Y.b. Lu] Will remove them. > > > + err = mc_send_command(mc_io, &cmd); > > + if (err) > > + return err; > > + > > + /* retrieve response parameters */ > > remove [Y.b. Lu] Will remove them. > > > + *token = mc_cmd_hdr_read_token(&cmd); > > + > > + return err; > > return 0; [Y.b. Lu] Wil fix it. > > > +} > > + > > +/** > > + * dprtc_close() - Close the control session of the object > > + * @mc_io: Pointer to MC portal's I/O object > > + * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_' > > + * @token: Token of DPRTC object > > + * > > + * After this function is called, no further operations are > > + * allowed on the object without opening a new control session. > > + * > > + * Return: '0' on Success; Error code otherwise. > > + */ > > +int dprtc_close(struct fsl_mc_io *mc_io, > > + u32 cmd_flags, > > + u16 token) > > +{ > > + struct fsl_mc_command cmd = { 0 }; > > + > > + /* prepare command */ > > + cmd.header = mc_encode_cmd_header(DPRTC_CMDID_CLOSE, > cmd_flags, > > + token); > > + > > + /* send command to mc*/ > > + return mc_send_command(mc_io, &cmd); } > > + > > +/** > > + * dprtc_create() - Create the DPRTC object. > > + * @mc_io: Pointer to MC portal's I/O object > > + * @dprc_token: Parent container token; '0' for default container > > + * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_' > > + * @cfg: Configuration structure > > + * @obj_id: Returned object id > > + * > > + * Create the DPRTC object, allocate required resources and > > + * perform required initialization. > > + * > > + * The function accepts an authentication token of a parent > > + * container that this object should be assigned to. The token > > + * can be '0' so the object will be assigned to the default container. > > + * The newly created object can be opened with the returned > > + * object id and using the container's associated tokens and MC portals. > > + * > > + * Return: '0' on Success; Error code otherwise. > > + */ > > +int dprtc_create(struct fsl_mc_io *mc_io, > > + u16 dprc_token, > > + u32 cmd_flags, > > + const struct dprtc_cfg *cfg, > > + u32 *obj_id) > > +{ > > + struct fsl_mc_command cmd = { 0 }; > > + int err; > > + > > + (void)(cfg); /* unused */ > > You can just remove these. Static checkers which complain about this are > stupid and a bit lazy. [Y.b. Lu] Will remove them. > > This driver seems nice and so far as I can see it doesn't need to be in staging > once we get the other parts merged. > > regards, > dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
