Hi Rob, Thanks for your review. > From: Rob Herring [mailto:robh@xxxxxxxxxx] > Sent: Tuesday, February 09, 2016 3:26 AM > To: Amitkumar Karwar > Cc: linux-wireless@xxxxxxxxxxxxxxx; Nishant Sarmukadam; > wnhuang@xxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Xinming Hu > Subject: Re: [PATCH v3 1/3] mwifiex: register platform specific driver > > On Mon, Feb 08, 2016 at 02:15:26AM -0800, Amitkumar Karwar wrote: > > From: Xinming Hu <huxm@xxxxxxxxxxx> > > > > Platform device and driver provides easy way to interact with > > device-tree-enabled system. > > > > This patch registers platform driver and reorganise existing device > > tree specific code. > > Corresponding device tree binding file is also created. > > Wrap you lines at 72 columns. Ack. > > Signed-off-by: Xinming Hu <huxm@xxxxxxxxxxx> > > Signed-off-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx> > > --- > > v3: Don't update adapter->dt_node if mwifiex_plt_dev is NULL > > --- > > Documentation/devicetree/bindings/mwifiex.txt | 29 +++++++++++ > > This doesn't belong at top level of bindings. bindings/net/wireless/ > > Also, mwifiex is a linux driver name. Name it after the chips. > marvell-sd8xxx? Sure. I will rename as marvell-sd8xxx and move it to bindings/net/wireless/ > > > drivers/net/wireless/marvell/mwifiex/Makefile | 1 + > > drivers/net/wireless/marvell/mwifiex/main.c | 2 + > > drivers/net/wireless/marvell/mwifiex/main.h | 14 +++++ > > .../net/wireless/marvell/mwifiex/platform_drv.c | 59 > ++++++++++++++++++++++ > > drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 6 +-- > > drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 2 +- > > 7 files changed, 109 insertions(+), 4 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/mwifiex.txt > > create mode 100644 > > drivers/net/wireless/marvell/mwifiex/platform_drv.c > > > > diff --git a/Documentation/devicetree/bindings/mwifiex.txt > > b/Documentation/devicetree/bindings/mwifiex.txt > > new file mode 100644 > > index 0000000..39b6a74 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mwifiex.txt > > @@ -0,0 +1,29 @@ > > +mwifiex > > +------ > > + > > +Required properties: > > + > > + - name : must be "mwifiex" > > + - compatible : must be "marvell,mwifiex" > > The naming should be named after the actual chip. Ok. How about having names as below? + - name : can be "sd8897", "sd8787" or "sd8797" + - compatible : must be "marvell,sd8xxx" > > > + > > +Optional properties: > > + > > + - mwifiex,caldata* : A series of properties with marvell,caldata > > + prefix, > > mwifiex is not a vendor. Got it. We will use "marvell,caldata*" here. > > > + represent Calibration data downloaded to the device > during > > + initialization. This is an array of unsigned values. > > + > > + > > +Example: > > + > > +Tx power limit calibration data is configured in below example. > > +The calibration data is an array of unsigned values, the length can > > +vary between hw versions. > > + > > +mwifiex { > > These chips are SDIO devices, right? This should be a child node of the > SDIO controller. > Yes. They are SDIO devices. We will have the node as child node of SDIO controller. Regards, Amitkumar Karwar -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
