From: ygardi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org To: Paul Bolle <pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org> Cc: Yaniv Gardi <ygardi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, james.bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, santoshsy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, linux-scsi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, subhashj-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, gbroner-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>, Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>, Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, Vinayak Holikatti <vinholikatti-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "James E.J. Bottomley" <jbottomley-wo1vFcy6AUs@public.gmane.org>, Dolev Raviv <draviv-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>, Sujit Reddy Thumma <sthumma-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, Raviv Shvili <rshvili-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, Sahitya Tummala <stummala-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, "open list:OPEN FIRMWARE AND..." <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH v2 4/4] scsi: ufs: probe and init of variant driver from the platform device Date: Sun, 7 Jun 2015 15:22:18 -0000 [thread overview] Message-ID: <9e35d6a7cdfd3d53510d41b0ff0a22fc.squirrel@www.codeaurora.org> (raw) In-Reply-To: <1433426833.15728.15.camel@x220> Thanks Paul for the review and comments. please see inline. > On Wed, 2015-06-03 at 12:37 +0300, Yaniv Gardi wrote: >> --- a/drivers/scsi/ufs/ufs-qcom.c >> +++ b/drivers/scsi/ufs/ufs-qcom.c > >> EXPORT_SYMBOL(ufs_hba_qcom_vops); > > Nothing uses this export. It's still a (static) symbol that is not > included in any header. I think this export serves no purpose. Am I > missing something subtle here? > you are correct. it will be removed. >> +/** >> + * ufs_qcom_probe - probe routine of the driver >> + * @pdev: pointer to Platform device handle >> + * >> + * Always return 0 >> + */ >> +static int ufs_qcom_probe(struct platform_device *pdev) >> +{ >> + dev_set_drvdata(&pdev->dev, (void *)&ufs_hba_qcom_vops); > > (Cast to void * should not be needed.) > when removing the cast there is compilation error: ufs-qcom.c: warning: passing argument 2 of dev_set_drvdata discards const qualifier from pointer target type error, forbidden warning: ufs-qcom.c >> + return 0; >> +} >> + >> +/** >> + * ufs_qcom_remove - set driver_data of the device to NULL >> + * @pdev: pointer to platform device handle >> + * >> + * Always return 0 >> + */ >> +static int ufs_qcom_remove(struct platform_device *pdev) >> +{ >> + dev_set_drvdata(&pdev->dev, NULL); >> + return 0; >> +} >> + >> +static const struct of_device_id ufs_qcom_of_match[] = { >> + { .compatible = "qcom,ufs_variant"}, >> + {}, >> +}; >> + >> +static struct platform_driver ufs_qcom_pltform = { >> + .probe = ufs_qcom_probe, >> + .remove = ufs_qcom_remove, >> + .driver = { >> + .name = "ufs_qcom", >> + .owner = THIS_MODULE, >> + .of_match_table = of_match_ptr(ufs_qcom_of_match), >> + }, >> +}; >> +module_platform_driver(ufs_qcom_pltform); > >> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > >> + struct device_node *node = pdev->dev.of_node; >> + struct device_node *ufs_variant_node; >> + struct platform_device *ufs_variant_pdev; > >> - hba->vops = get_variant_ops(&pdev->dev); >> + err = of_platform_populate(node, NULL, NULL, &pdev->dev); >> + if (err) >> + dev_err(&pdev->dev, >> + "%s: of_platform_populate() failed\n", __func__); >> + >> + ufs_variant_node = of_get_next_available_child(node, NULL); >> + >> + if (!ufs_variant_node) { >> + dev_dbg(&pdev->dev, "failed to find ufs_variant_node child\n"); >> + } else { >> + ufs_variant_pdev = of_find_device_by_node(ufs_variant_node); >> + >> + if (ufs_variant_pdev) >> + hba->vops = (struct ufs_hba_variant_ops *) >> + dev_get_drvdata(&ufs_variant_pdev->dev); > > (Another cast that I think is not needed.) here you are right - this one can be removed and will be > >> + } > > If I scanned this correctly, the dev_set_drvdata() and dev_get_drvdata() > pair adds an actual user of ufs_hba_qcom_vops. So that ends the obvious > issue I think the code currently has. And I gladly defer to the scsi > people to determine whether that is done the right way. > > Thanks, > > > Paul Bolle > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: ygardi@codeaurora.org To: "Paul Bolle" <pebolle@tiscali.nl> Cc: "Yaniv Gardi" <ygardi@codeaurora.org>, james.bottomley@hansenpartnership.com, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, linux-arm-msm@vger.kernel.org, santoshsy@gmail.com, linux-scsi-owner@vger.kernel.org, subhashj@codeaurora.org, gbroner@codeaurora.org, "Rob Herring" <robh+dt@kernel.org>, "Pawel Moll" <pawel.moll@arm.com>, "Mark Rutland" <mark.rutland@arm.com>, "Ian Campbell" <ijc+devicetree@hellion.org.uk>, "Kumar Gala" <galak@codeaurora.org>, "Vinayak Holikatti" <vinholikatti@gmail.com>, "James E.J. Bottomley" <jbottomley@odin.com>, "Dolev Raviv" <draviv@codeaurora.org>, "Christoph Hellwig" <hch@lst.de>, "Sujit Reddy Thumma" <sthumma@codeaurora.org>, "Raviv Shvili" <rshvili@codeaurora.org>, "Sahitya Tummala" <stummala@codeaurora.org>, "open list:OPEN FIRMWARE AND..." <devicetree@vger.kernel.org> Subject: Re: [PATCH v2 4/4] scsi: ufs: probe and init of variant driver from the platform device Date: Sun, 7 Jun 2015 15:22:18 -0000 [thread overview] Message-ID: <9e35d6a7cdfd3d53510d41b0ff0a22fc.squirrel@www.codeaurora.org> (raw) In-Reply-To: <1433426833.15728.15.camel@x220> Thanks Paul for the review and comments. please see inline. > On Wed, 2015-06-03 at 12:37 +0300, Yaniv Gardi wrote: >> --- a/drivers/scsi/ufs/ufs-qcom.c >> +++ b/drivers/scsi/ufs/ufs-qcom.c > >> EXPORT_SYMBOL(ufs_hba_qcom_vops); > > Nothing uses this export. It's still a (static) symbol that is not > included in any header. I think this export serves no purpose. Am I > missing something subtle here? > you are correct. it will be removed. >> +/** >> + * ufs_qcom_probe - probe routine of the driver >> + * @pdev: pointer to Platform device handle >> + * >> + * Always return 0 >> + */ >> +static int ufs_qcom_probe(struct platform_device *pdev) >> +{ >> + dev_set_drvdata(&pdev->dev, (void *)&ufs_hba_qcom_vops); > > (Cast to void * should not be needed.) > when removing the cast there is compilation error: ufs-qcom.c: warning: passing argument 2 of dev_set_drvdata discards const qualifier from pointer target type error, forbidden warning: ufs-qcom.c >> + return 0; >> +} >> + >> +/** >> + * ufs_qcom_remove - set driver_data of the device to NULL >> + * @pdev: pointer to platform device handle >> + * >> + * Always return 0 >> + */ >> +static int ufs_qcom_remove(struct platform_device *pdev) >> +{ >> + dev_set_drvdata(&pdev->dev, NULL); >> + return 0; >> +} >> + >> +static const struct of_device_id ufs_qcom_of_match[] = { >> + { .compatible = "qcom,ufs_variant"}, >> + {}, >> +}; >> + >> +static struct platform_driver ufs_qcom_pltform = { >> + .probe = ufs_qcom_probe, >> + .remove = ufs_qcom_remove, >> + .driver = { >> + .name = "ufs_qcom", >> + .owner = THIS_MODULE, >> + .of_match_table = of_match_ptr(ufs_qcom_of_match), >> + }, >> +}; >> +module_platform_driver(ufs_qcom_pltform); > >> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > >> + struct device_node *node = pdev->dev.of_node; >> + struct device_node *ufs_variant_node; >> + struct platform_device *ufs_variant_pdev; > >> - hba->vops = get_variant_ops(&pdev->dev); >> + err = of_platform_populate(node, NULL, NULL, &pdev->dev); >> + if (err) >> + dev_err(&pdev->dev, >> + "%s: of_platform_populate() failed\n", __func__); >> + >> + ufs_variant_node = of_get_next_available_child(node, NULL); >> + >> + if (!ufs_variant_node) { >> + dev_dbg(&pdev->dev, "failed to find ufs_variant_node child\n"); >> + } else { >> + ufs_variant_pdev = of_find_device_by_node(ufs_variant_node); >> + >> + if (ufs_variant_pdev) >> + hba->vops = (struct ufs_hba_variant_ops *) >> + dev_get_drvdata(&ufs_variant_pdev->dev); > > (Another cast that I think is not needed.) here you are right - this one can be removed and will be > >> + } > > If I scanned this correctly, the dev_set_drvdata() and dev_get_drvdata() > pair adds an actual user of ufs_hba_qcom_vops. So that ends the obvious > issue I think the code currently has. And I gladly defer to the scsi > people to determine whether that is done the right way. > > Thanks, > > > Paul Bolle > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
next prev parent reply other threads:[~2015-06-07 15:22 UTC|newest] Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-06-03 9:37 [PATCH v2 0/4] fixing building errors and warnings when components Yaniv Gardi 2015-06-03 9:37 ` [PATCH v2 1/4] phy: qcom-ufs: fix build error when the component is built as a module Yaniv Gardi 2015-06-03 9:37 ` [PATCH v2 2/4] scsi: ufs-qcom: fix compilation warning if compiled " Yaniv Gardi 2015-06-03 9:37 ` [PATCH v2 3/4] scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component Yaniv Gardi 2015-06-03 9:37 ` [PATCH v2 4/4] scsi: ufs: probe and init of variant driver from the platform device Yaniv Gardi 2015-06-03 9:37 ` Yaniv Gardi 2015-06-04 14:07 ` Paul Bolle 2015-06-04 14:42 ` Paul Bolle 2015-06-04 20:42 ` ygardi 2015-06-07 15:22 ` ygardi-sgV2jX0FEOL9JmXXK+q4OQ [this message] 2015-06-07 15:22 ` ygardi 2015-06-04 14:32 ` Akinobu Mita 2015-06-04 14:32 ` Akinobu Mita 2015-06-04 20:53 ` ygardi 2015-06-04 20:53 ` ygardi 2015-06-05 16:47 ` Akinobu Mita 2015-06-05 16:47 ` Akinobu Mita 2015-06-07 15:32 ` ygardi 2015-06-07 15:32 ` ygardi 2015-06-08 14:47 ` Akinobu Mita 2015-06-08 14:47 ` Akinobu Mita 2015-06-08 15:02 ` Rob Herring 2015-06-08 15:02 ` Rob Herring 2015-06-09 5:53 ` Dov Levenglick 2015-06-09 5:53 ` Dov Levenglick [not found] ` <bfade46f9d953e10240acb835105b81d.squirrel-mMfbam+mt9083fI46fginR2eb7JE58TQ@public.gmane.org> 2015-06-09 12:53 ` Rob Herring 2015-06-09 12:53 ` Rob Herring 2015-06-17 7:42 ` Dov Levenglick 2015-06-17 7:42 ` Dov Levenglick 2015-06-17 12:46 ` Rob Herring 2015-06-17 12:46 ` Rob Herring 2015-06-17 13:17 ` Dov Levenglick 2015-06-17 13:17 ` Dov Levenglick 2015-06-17 13:37 ` Rob Herring 2015-06-17 13:37 ` Rob Herring 2015-06-17 14:21 ` Dov Levenglick 2015-06-17 14:21 ` Dov Levenglick 2015-06-17 14:31 ` James Bottomley 2015-06-17 14:31 ` James Bottomley 2015-06-17 14:38 ` Dov Levenglick 2015-06-17 14:38 ` Dov Levenglick [not found] ` <CAC5umyis6HU06KU-aSGoy6mCt-u+5Pz16nDk1qHrcsTnFo_2hA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-06-08 14:51 ` Rob Herring 2015-06-08 14:51 ` Rob Herring
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=9e35d6a7cdfd3d53510d41b0ff0a22fc.squirrel@www.codeaurora.org \ --to=ygardi-sgv2jx0feol9jmxxk+q4oq@public.gmane.org \ --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=draviv-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=gbroner-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=hch-jcswGhMUV9g@public.gmane.org \ --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \ --cc=james.bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org \ --cc=jbottomley-wo1vFcy6AUs@public.gmane.org \ --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-scsi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \ --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \ --cc=pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org \ --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ --cc=rshvili-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=santoshsy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=sthumma-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=stummala-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=subhashj-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=vinholikatti-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ /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: linkBe 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.