Re: [PATCH] staging: rtl8188eu: os_dep: usb_intf.c: Fix for possible null pointer dereference
On Tue, May 20, 2014 at 06:26:51PM -0500, Larry Finger wrote: > On 05/20/2014 04:31 PM, Rickard Strandqvist wrote: > >There is otherwise a risk of a possible null pointer dereference. > > > >Was largely found by using a static code analysis program called cppcheck. > > > >Signed-off-by: Rickard Strandqvist > >--- > > drivers/staging/rtl8188eu/os_dep/usb_intf.c | 127 > > ++- > > 1 file changed, 66 insertions(+), 61 deletions(-) > > Although cppcheck's analysis is correct, pointer padapter can never > be null in any of these routines. In routine rtw_drv_init(), > rtw_usb_if1_init() is called to allocate memory for struct adapter > for the driver. If that fails, none of these other routines will > ever be called as the driver will not be loaded. > > If it is deemed better to fill the code with needless tests because > some static checker points out a place like this, that is OK with > me, but I do not see the point. If it's an invariant of the code that padapter cannot ever be NULL, then that seems perfectly fine to rely on, and the tool just needs fixing or silencing. At most, it might be helpful to add annotations like GCC's "nonnull", if that helps the checker and the compiler generate more useful warnings. - Josh Triplett ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging/comedi: Fixes static analysis warning raised by sparse
On Thu, Jun 12, 2014 at 12:24:25AM +0300, Dan Carpenter wrote:
> Let's forward this to the Sparse mailing list.
>
> We're seeing a Sparse false positive testing
> drivers/staging/comedi/drivers/ni_pcimio.c.
>
> CHECK drivers/staging/comedi/drivers/ni_pcimio.c
> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> (4294967295) for type int
> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> (4294967295) for type int
> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> (4294967295) for type int
> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> (4294967295) for type int
>
> I have created some test code to demonstrate the problem (attached).
>
> The check_shift_count() warning is only supposed to be printed for
> number literals but because of the way inline functions are expanded it
> still complains even though channel is a variable.
Thanks for the test case; this definitely makes no sense. I don't think
Sparse will suddenly develop enough range analysis or reachability
analysis to handle this case; I think the right answer is to avoid
giving such warnings for shifts with a non-constant RHS.
> #include
> #include
> #include
>
> static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
> {
> if (channel < 4)
> return 1 << channel;
> return 0;
> }
>
> static inline void filter(int channel)
> {
> if (channel < 0)
> return;
> ni_stc_dma_channel_select_bitfield(channel);
> }
>
> int main(void)
> {
> filter(-1);
>
> return 0;
> }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/8] wlan-ng/prism2mgmt:checkpatch: Fix string split
On Thu, Jun 19, 2014 at 09:20:14PM +0200, Johannes Stadlinger wrote:
> This patch fixes all warnings of checkpatch about string splitting.
>
> Signed-off-by: Johannes Stadlinger
> Signed-off-by: Maximilian Eschenbacher
> CC: linux-ker...@i4.cs.fau.de
> CC: Greg Kroah-Hartman
> CC: Himangi Saraogi
> CC: Josh Triplett
> CC: Vitaly Osipov
> CC: de...@driverdev.osuosl.org
> CC: linux-ker...@vger.kernel.org
Reviewed-by: Josh Triplett
> drivers/staging/wlan-ng/prism2mgmt.c | 26 ++
> 1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/wlan-ng/prism2mgmt.c
> b/drivers/staging/wlan-ng/prism2mgmt.c
> index 36a3e1a..f90f7da 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.c
> +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> @@ -178,8 +178,7 @@ int prism2mgmt_scan(wlandevice_t *wlandev, void *msgp)
>word);
> if (result) {
> netdev_warn(wlandev->netdev,
> - "Passive scan not supported with "
> - "current firmware. (<1.5.1)\n");
> + "Passive scan not supported with current
> firmware. (<1.5.1)\n");
> }
> }
>
> @@ -381,8 +380,7 @@ int prism2mgmt_scan_results(wlandevice_t *wlandev, void
> *msgp)
>
> if (!hw->scanresults) {
> netdev_err(wlandev->netdev,
> -"dot11req_scan_results can only be used after "
> -"a successful dot11req_scan.\n");
> +"dot11req_scan_results can only be used after a
> successful dot11req_scan.\n");
> result = 2;
> req->resultcode.data = P80211ENUM_resultcode_invalid_parameters;
> goto exit;
> @@ -733,8 +731,8 @@ int prism2mgmt_readpda(wlandevice_t *wlandev, void *msgp)
> HFA384x_PDA_LEN_MAX);
> if (result) {
> netdev_err(wlandev->netdev,
> -"hfa384x_drvr_readpda() failed, "
> -"result=%d\n", result);
> +"hfa384x_drvr_readpda() failed, result=%d\n",
> +result);
>
> msg->resultcode.data =
> P80211ENUM_resultcode_implementation_failure;
> @@ -782,8 +780,7 @@ int prism2mgmt_ramdl_state(wlandevice_t *wlandev, void
> *msgp)
>
> if (wlandev->msdstate != WLAN_MSD_FWLOAD) {
> netdev_err(wlandev->netdev,
> -"ramdl_state(): may only be called "
> -"in the fwload state.\n");
> +"ramdl_state(): may only be called in the fwload
> state.\n");
> msg->resultcode.data =
> P80211ENUM_resultcode_implementation_failure;
> msg->resultcode.status = P80211ENUM_msgitem_status_data_ok;
> @@ -841,8 +838,7 @@ int prism2mgmt_ramdl_write(wlandevice_t *wlandev, void
> *msgp)
>
> if (wlandev->msdstate != WLAN_MSD_FWLOAD) {
> netdev_err(wlandev->netdev,
> -"ramdl_write(): may only be called "
> -"in the fwload state.\n");
> +"ramdl_write(): may only be called in the fwload
> state.\n");
> msg->resultcode.data =
> P80211ENUM_resultcode_implementation_failure;
> msg->resultcode.status = P80211ENUM_msgitem_status_data_ok;
> @@ -901,8 +897,7 @@ int prism2mgmt_flashdl_state(wlandevice_t *wlandev, void
> *msgp)
>
> if (wlandev->msdstate != WLAN_MSD_FWLOAD) {
> netdev_err(wlandev->netdev,
> -"flashdl_state(): may only be called "
> -"in the fwload state.\n");
> +"flashdl_state(): may only be called in the fwload
> state.\n");
> msg->resultcode.data =
> P80211ENUM_resultcode_implementation_failure;
> msg->resultcode.status = P80211ENUM_msgitem_status_data_ok;
> @@ -936,8 +931,8 @@ int prism2mgmt_flashdl_state(wlandevice_t *wlandev, void
> *msgp)
> result = prism2sta_ifstate(wlandev, P80211ENUM_ifstate_fwload);
> if (result != P80211ENUM_resultcode_success) {
> netdev_err(wlandev->netdev,
> -
Re: [PATCH 3/8] wlan-ng/prism2mgmt:checkpatch: Insert blank line
On Thu, Jun 19, 2014 at 09:20:15PM +0200, Johannes Stadlinger wrote:
> This patch inserts a blank line after a declaration to avoid checkpatch
> warning.
>
> Signed-off-by: Johannes Stadlinger
> Signed-off-by: Maximilian Eschenbacher
> CC: linux-ker...@i4.cs.fau.de
> CC: Greg Kroah-Hartman
> CC: Josh Triplett
> CC: Himangi Saraogi
> CC: Vitaly Osipov
> CC: de...@driverdev.osuosl.org
> CC: linux-ker...@vger.kernel.org
This is one case where I think checkpatch is just wrong; this doesn't
make the code any more (or less) readable.
Meh-by: Josh Triplett
> drivers/staging/wlan-ng/prism2mgmt.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/wlan-ng/prism2mgmt.c
> b/drivers/staging/wlan-ng/prism2mgmt.c
> index f90f7da..e6a82d3 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.c
> +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> @@ -190,6 +190,7 @@ int prism2mgmt_scan(wlandevice_t *wlandev, void *msgp)
> word = 0;
> for (i = 0; i < msg->channellist.data.len; i++) {
> u8 channel = msg->channellist.data.data[i];
> +
> if (channel > 14)
> continue;
> /* channel 1 is BIT 0 ... channel 14 is BIT 13 */
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/8] wlan-ng/prism2mib:checkpatch: Fix string split
On Thu, Jun 19, 2014 at 09:20:16PM +0200, Johannes Stadlinger wrote:
> This patch fixes a warning of checkpatch about string splitting.
>
> Signed-off-by: Johannes Stadlinger
> Signed-off-by: Maximilian Eschenbacher
> CC: linux-ker...@i4.cs.fau.de
> CC: Greg Kroah-Hartman
> CC: Josh Triplett
> CC: Vitaly Osipov
> CC: Himangi Saraogi
> CC: de...@driverdev.osuosl.org
> CC: linux-ker...@vger.kernel.org
Reviewed-by: Josh Triplett
> drivers/staging/wlan-ng/prism2mib.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/wlan-ng/prism2mib.c
> b/drivers/staging/wlan-ng/prism2mib.c
> index 0fb42df..bdd3b4c 100644
> --- a/drivers/staging/wlan-ng/prism2mib.c
> +++ b/drivers/staging/wlan-ng/prism2mib.c
> @@ -672,8 +672,8 @@ static int prism2mib_fragmentationthreshold(struct mibrec
> *mib,
>
> if (!isget)
> if ((*uint32) % 2) {
> - netdev_warn(wlandev->netdev, "Attempt to set odd number
> "
> -"FragmentationThreshold\n");
> + netdev_warn(wlandev->netdev,
> + "Attempt to set odd number
> FragmentationThreshold\n");
> msg->resultcode.data =
> P80211ENUM_resultcode_not_supported;
> return 0;
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/8] wlan-ng/prism2mib:checkpatch: Insert blank lines
On Thu, Jun 19, 2014 at 09:20:17PM +0200, Johannes Stadlinger wrote:
> This patch inserts blank lines after declarations to avoid checkpatch
> warning.
>
> After our fixes in 'wlan-ng/prism2mib.c' there are still two checkpatch
> warnings about lines over 80 characters remaining.
>
> Signed-off-by: Johannes Stadlinger
> Signed-off-by: Maximilian Eschenbacher
> CC: linux-ker...@i4.cs.fau.de
> CC: Greg Kroah-Hartman
> CC: Josh Triplett
> CC: Vitaly Osipov
> CC: Himangi Saraogi
> CC: de...@driverdev.osuosl.org
> CC: linux-ker...@vger.kernel.org
> ---
> drivers/staging/wlan-ng/prism2mib.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wlan-ng/prism2mib.c
> b/drivers/staging/wlan-ng/prism2mib.c
> index bdd3b4c..3ea24d6 100644
> --- a/drivers/staging/wlan-ng/prism2mib.c
> +++ b/drivers/staging/wlan-ng/prism2mib.c
> @@ -85,7 +85,8 @@ struct mibrec {
> u16 parm1;
> u16 parm2;
> u16 parm3;
> - int (*func) (struct mibrec *mib,
> +
> + int (*func)(struct mibrec *mib,
Eliminating the space here makes sense, but checkpatch shouldn't warn
about spaces after declarations between two fields in the middle of a
structure declaration.
>int isget,
>wlandevice_t *wlandev,
>hfa384x_t *hw,
> @@ -722,6 +723,7 @@ static int prism2mib_priv(struct mibrec *mib,
> switch (mib->did) {
> case DIDmib_lnx_lnxConfigTable_lnxRSNAIE:{
> hfa384x_WPAData_t wpa;
> +
> if (isget) {
> hfa384x_drvr_getconfig(hw,
> HFA384x_RID_CNFWPADATA,
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/8] wlan-ng/prism2sta:checkpatch: Fix long lines
On Thu, Jun 19, 2014 at 09:20:18PM +0200, Johannes Stadlinger wrote: > This patch fixes all warning of checkpatch about lines over 80 > characters. > > Signed-off-by: Johannes Stadlinger > Signed-off-by: Maximilian Eschenbacher > CC: linux-ker...@i4.cs.fau.de > CC: Greg Kroah-Hartman > CC: Tugce Sirin > CC: Josh Triplett > CC: Neil Armstrong > CC: Paul Gortmaker > CC: Vitaly Osipov > CC: de...@driverdev.osuosl.org > CC: linux-ker...@vger.kernel.org Most of these look fine, but... > @@ -1271,7 +1275,8 @@ void prism2sta_processing_defer(struct work_struct > *data) >HFA384x_RID_CURRENTSSID, result); > return; > } > - prism2mgmt_bytestr2pstr((struct hfa384x_bytestr *) > &ssid, > + prism2mgmt_bytestr2pstr((struct hfa384x_bytestr *) > + &ssid, > (p80211pstrd_t *) & > wlandev->ssid); ...that (and the one in the subsequent context lines) looks horrible. I'd suggest breaking after the opening parenthesis of the function call and just indenting the arguments by one tab: foo_with_long_name( long_arg, another_long_arg); Better yet, ignore checkpatch and format that call more readably as though the 80 column rule didn't exist. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/8] wlan-ng/prism2sta:checkpatch: Fix string split
On Thu, Jun 19, 2014 at 09:20:19PM +0200, Johannes Stadlinger wrote: > This patch fixes a warning of checkpatch about string splitting. > > Signed-off-by: Johannes Stadlinger > Signed-off-by: Maximilian Eschenbacher > CC: linux-ker...@i4.cs.fau.de > CC: Greg Kroah-Hartman > CC: Tugce Sirin > CC: Josh Triplett > CC: Vitaly Osipov > CC: Neil Armstrong > CC: Paul Gortmaker > CC: de...@driverdev.osuosl.org > CC: linux-ker...@vger.kernel.org Reviewed-by: Josh Triplett > drivers/staging/wlan-ng/prism2sta.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/wlan-ng/prism2sta.c > b/drivers/staging/wlan-ng/prism2sta.c > index a449fdf..9444006 100644 > --- a/drivers/staging/wlan-ng/prism2sta.c > +++ b/drivers/staging/wlan-ng/prism2sta.c > @@ -468,8 +468,7 @@ u32 prism2sta_ifstate(wlandevice_t *wlandev, u32 ifstate) > break; > case WLAN_MSD_RUNNING: > netdev_warn(wlandev->netdev, > -"Cannot enter fwload state from enable state," > -"you must disable first.\n"); > + "Cannot enter fwload state from enable > state, you must disable first.\n"); > result = P80211ENUM_resultcode_invalid_parameters; > break; > case WLAN_MSD_HWFAIL: > -- > 1.9.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 8/8] wlan-ng/prism2sta:checkpatch: Insert blank lines
On Thu, Jun 19, 2014 at 09:20:20PM +0200, Johannes Stadlinger wrote:
> This patch inserts blank lines after declarations to avoid checkpatch
> warnings.
>
> After our fixes in 'wlan-ng/prism2sta' there is still a checkpatch
> warning about prefering 'ether_addr_copy' instead of 'memcpy'
> remaining.
>
> Signed-off-by: Johannes Stadlinger
> Signed-off-by: Maximilian Eschenbacher
> CC: linux-ker...@i4.cs.fau.de
> CC: Greg Kroah-Hartman
> CC: Tugce Sirin
> CC: Josh Triplett
> CC: Himangi Saraogi
> CC: Paul Gortmaker
> CC: Vitaly Osipov
> CC: Neil Armstrong
> CC: de...@driverdev.osuosl.org
> CC: linux-ker...@vger.kernel.org
This one does look like an improvement.
Reviewed-by: Josh Triplett
> drivers/staging/wlan-ng/prism2sta.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/wlan-ng/prism2sta.c
> b/drivers/staging/wlan-ng/prism2sta.c
> index 9444006..8d4d7ba 100644
> --- a/drivers/staging/wlan-ng/prism2sta.c
> +++ b/drivers/staging/wlan-ng/prism2sta.c
> @@ -360,6 +360,7 @@ static int prism2sta_mlmerequest(wlandevice_t *wlandev,
> struct p80211msg *msg)
> case DIDmsg_lnxreq_ifstate:
> {
> struct p80211msg_lnxreq_ifstate *ifstatemsg;
> +
> pr_debug("Received mlme ifstate request\n");
> ifstatemsg = (struct p80211msg_lnxreq_ifstate *) msg;
> result =
> @@ -1411,6 +1412,7 @@ void prism2sta_processing_defer(struct work_struct
> *data)
>*/
> if (hw->join_ap && --hw->join_retries > 0) {
> hfa384x_JoinRequest_data_t joinreq;
> +
> joinreq = hw->joinreq;
> /* Send the join request */
> hfa384x_drvr_setconfig(hw,
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] Staging: bcm: Misc whitespace fixes
On Mon, Jun 23, 2014 at 11:42:28AM +0200, Matthias Beyer wrote:
> Signed-off-by: Matthias Beyer
Reviewed-by: Josh Triplett
> drivers/staging/bcm/DDRInit.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/bcm/DDRInit.c b/drivers/staging/bcm/DDRInit.c
> index f1d7cb8..b4f0ea3 100644
> --- a/drivers/staging/bcm/DDRInit.c
> +++ b/drivers/staging/bcm/DDRInit.c
> @@ -776,7 +776,7 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> struct bcm_ddr_setting *psDDRSetting = NULL;
> ULONG RegCount = 0;
> UINT value = 0;
> - UINT uiResetValue = 0;
> + UINT uiResetValue = 0;
> UINT uiClockSetting = 0;
> int retval = STATUS_SUCCESS;
>
> @@ -1068,7 +1068,7 @@ int download_ddr_settings(struct bcm_mini_adapter
> *Adapter)
> struct bcm_ddr_setting *psDDRSetting = NULL;
> ULONG RegCount = 0;
> unsigned long ul_ddr_setting_load_addr =
> DDR_DUMP_INTERNAL_DEVICE_MEMORY;
> - UINT value = 0;
> + UINT value = 0;
> int retval = STATUS_SUCCESS;
> bool bOverrideSelfRefresh = false;
>
> @@ -1222,9 +1222,9 @@ int download_ddr_settings(struct bcm_mini_adapter
> *Adapter)
> break;
> }
> } else {
> - value = psDDRSetting->ulRegValue;
> + value = psDDRSetting->ulRegValue;
>
> - if (STATUS_SUCCESS != wrmalt(Adapter,
> ul_ddr_setting_load_addr ,
> + if (STATUS_SUCCESS != wrmalt(Adapter,
> ul_ddr_setting_load_addr,
> &value, sizeof(value))) {
> BCM_DEBUG_PRINT(Adapter,
> DBG_TYPE_PRINTK, 0, 0, "%s:%d\n", __func__, __LINE__);
> break;
> @@ -1235,5 +1235,5 @@ int download_ddr_settings(struct bcm_mini_adapter
> *Adapter)
> RegCount--;
> psDDRSetting++;
> }
> - return retval;
> + return retval;
> }
> --
> 2.0.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] Staging: bcm: Indentation fixes
On Mon, Jun 23, 2014 at 11:42:29AM +0200, Matthias Beyer wrote:
> This patch fixes some indentation errors, where multi-line statements
> where not indented.
>
> Signed-off-by: Matthias Beyer
This patch also fixes some whitespace issues, such as a lack of space
before '/', which should probably go into the previous patch.
> drivers/staging/bcm/DDRInit.c | 56
> +--
> 1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/bcm/DDRInit.c b/drivers/staging/bcm/DDRInit.c
> index b4f0ea3..4c7f518 100644
> --- a/drivers/staging/bcm/DDRInit.c
> +++ b/drivers/staging/bcm/DDRInit.c
> @@ -785,18 +785,18 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> switch (Adapter->DDRSetting) {
> case DDR_80_MHZ:
> psDDRSetting = asT3LP_DDRSetting80MHz;
> - RegCount = (sizeof(asT3LP_DDRSetting80MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3LP_DDRSetting80MHz) /
> + sizeof(struct bcm_ddr_setting));
> break;
> case DDR_100_MHZ:
> psDDRSetting = asT3LP_DDRSetting100MHz;
> - RegCount = (sizeof(asT3LP_DDRSetting100MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3LP_DDRSetting100MHz) /
> + sizeof(struct bcm_ddr_setting));
> break;
> case DDR_133_MHZ:
> psDDRSetting = asT3LP_DDRSetting133MHz;
> - RegCount = (sizeof(asT3LP_DDRSetting133MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3LP_DDRSetting133MHz) /
> + sizeof(struct bcm_ddr_setting));
> if (Adapter->bMipsConfig == MIPS_200_MHZ)
> uiClockSetting = 0x03F13652;
> else
> @@ -836,18 +836,18 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
>
> case DDR_80_MHZ:
> psDDRSetting = asT3LPB_DDRSetting80MHz;
> - RegCount = (sizeof(asT3B_DDRSetting80MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3B_DDRSetting80MHz) /
> + sizeof(struct bcm_ddr_setting));
> break;
> case DDR_100_MHZ:
> psDDRSetting = asT3LPB_DDRSetting100MHz;
> - RegCount = (sizeof(asT3B_DDRSetting100MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3B_DDRSetting100MHz) /
> + sizeof(struct bcm_ddr_setting));
> break;
> case DDR_133_MHZ:
> psDDRSetting = asT3LPB_DDRSetting133MHz;
> - RegCount = (sizeof(asT3B_DDRSetting133MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3B_DDRSetting133MHz) /
> + sizeof(struct bcm_ddr_setting));
>
> if (Adapter->bMipsConfig == MIPS_200_MHZ)
> uiClockSetting = 0x03F13652;
> @@ -875,18 +875,18 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> switch (Adapter->DDRSetting) {
> case DDR_80_MHZ:
> psDDRSetting = asT3_DDRSetting80MHz;
> - RegCount = (sizeof(asT3_DDRSetting80MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3_DDRSetting80MHz) /
> + sizeof(struct bcm_ddr_setting));
> break;
> case DDR_100_MHZ:
> psDDRSetting = asT3_DDRSetting100MHz;
> - RegCount = (sizeof(asT3_DDRSetting100MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3_DDRSetting100MHz) /
> + sizeof(struct bcm_ddr_setting));
> break;
> case DDR_133_MHZ:
> psDDRSetting = asT3_DDRSetting133MHz;
> - RegCount = (sizeof(asT3_DDRSetting133MHz)/
> - sizeof(struct bcm_ddr_setting));
> + RegCount = (sizeof(asT3_DDRSetting133MHz) /
> + sizeof(struct bcm_ddr_setting));
> break;
> default:
> return -EINVAL;
> @@ -896,26 +896,26 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> switch (Adapter->DDRSetting) {
> case DDR_80_MHZ:
> psDDRSetting = asT3B_DDRSetting80MHz;
> -
Re: [PATCH 3/6] Staging: bcm: Lines shortened in ddr_init()
On Mon, Jun 23, 2014 at 11:42:30AM +0200, Matthias Beyer wrote:
> Signed-off-by: Matthias Beyer
Honestly, none of these seem like improvements. Checkpatch's line-width
warnings rarely result in improved code.
(Its "maximum indentation level" warnings, which prompt people to
refactor functions, can sometimes help; however, given a sensible
indentation level, wrapping lines rarely if ever seems like an
improvement.)
> drivers/staging/bcm/DDRInit.c | 176
> --
> 1 file changed, 134 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/staging/bcm/DDRInit.c b/drivers/staging/bcm/DDRInit.c
> index 4c7f518..cfaa2c1 100644
> --- a/drivers/staging/bcm/DDRInit.c
> +++ b/drivers/staging/bcm/DDRInit.c
> @@ -818,15 +818,21 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> if ((Adapter->chip_id != BCS220_2) &&
> (Adapter->chip_id != BCS220_2BC) &&
> (Adapter->chip_id != BCS220_3)) {
> - retval = rdmalt(Adapter, (UINT)0x0f000830, &uiResetValue,
> sizeof(uiResetValue));
> + retval = rdmalt(Adapter, (UINT)0x0f000830, &uiResetValue,
> + sizeof(uiResetValue));
> if (retval < 0) {
> - BCM_DEBUG_PRINT(Adapter, CMHOST, RDM, DBG_LVL_ALL,
> "%s:%d RDM failed\n", __func__, __LINE__);
> + BCM_DEBUG_PRINT(Adapter, CMHOST, RDM, DBG_LVL_ALL,
> + "%s:%d RDM failed\n",
> + __func__, __LINE__);
> return retval;
> }
> uiResetValue |= 0x44;
> - retval = wrmalt(Adapter, (UINT)0x0f000830, &uiResetValue,
> sizeof(uiResetValue));
> + retval = wrmalt(Adapter, (UINT)0x0f000830, &uiResetValue,
> + sizeof(uiResetValue));
> if (retval < 0) {
> - BCM_DEBUG_PRINT(Adapter, CMHOST, RDM, DBG_LVL_ALL,
> "%s:%d RDM failed\n", __func__, __LINE__);
> + BCM_DEBUG_PRINT(Adapter, CMHOST, RDM, DBG_LVL_ALL,
> + "%s:%d RDM failed\n",
> + __func__, __LINE__);
> return retval;
> }
> }
> @@ -857,7 +863,8 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
>
> case DDR_160_MHZ:
> psDDRSetting = asT3LPB_DDRSetting160MHz;
> - RegCount =
> sizeof(asT3LPB_DDRSetting160MHz)/sizeof(struct bcm_ddr_setting);
> + RegCount = sizeof(asT3LPB_DDRSetting160MHz) /
> +sizeof(struct bcm_ddr_setting);
>
> if (Adapter->bMipsConfig == MIPS_200_MHZ)
> uiClockSetting = 0x03F137D2;
> @@ -871,7 +878,8 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> case 0xbece0121:
> case 0xbece0130:
> case 0xbece0300:
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, DRV_ENTRY, DBG_LVL_ALL,
> "DDR Setting: %x\n", Adapter->DDRSetting);
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, DRV_ENTRY, DBG_LVL_ALL,
> + "DDR Setting: %x\n", Adapter->DDRSetting);
> switch (Adapter->DDRSetting) {
> case DDR_80_MHZ:
> psDDRSetting = asT3_DDRSetting80MHz;
> @@ -933,15 +941,19 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> }
>
> value = 0;
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, DRV_ENTRY, DBG_LVL_ALL,
> "Register Count is =%lu\n", RegCount);
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, DRV_ENTRY, DBG_LVL_ALL,
> + "Register Count is =%lu\n", RegCount);
> while (RegCount && !retval) {
> - if (uiClockSetting && psDDRSetting->ulRegAddress ==
> MIPS_CLOCK_REG)
> + if (uiClockSetting
> + && psDDRSetting->ulRegAddress == MIPS_CLOCK_REG)
> value = uiClockSetting;
> else
> value = psDDRSetting->ulRegValue;
> - retval = wrmalt(Adapter, psDDRSetting->ulRegAddress, &value,
> sizeof(value));
> + retval = wrmalt(Adapter, psDDRSetting->ulRegAddress, &value,
> + sizeof(value));
> if (STATUS_SUCCESS != retval) {
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> "%s:%d\n", __func__, __LINE__);
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> + "%s:%d\n", __func__, __LINE__);
> break;
> }
>
> @@ -957,27 +969,47 @@ int ddr_init(struct bcm_mini_adapter *Adapter)
> (Adapter->chip_id != BCS220_3)) {
> /* drive MDDR to half in case of UMA-B: */
> uiResetValue = 0x01010001;
> - retval = wrmalt(Adapter, (UINT)0x0F0
Re: [PATCH 4/6] Staging: bcm: Fixed indention for inner if-block
On Mon, Jun 23, 2014 at 11:42:31AM +0200, Matthias Beyer wrote:
> The inner if-statement was aligned just like the outer one. Why?
>
> This indention was introduced by
>
> f34c488c3894968e8cdbdc3b1ed617d78315cace
>
> which is a indention-fix patch itself. That's why I'm curious about it.
>
> I did not merge these nested if-statements, as I don't know if I'm
> destroying logical seperated checks with it.
>
> Signed-off-by: Matthias Beyer
Reviewed-by: Josh Triplett
> drivers/staging/bcm/DDRInit.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/bcm/DDRInit.c b/drivers/staging/bcm/DDRInit.c
> index cfaa2c1..d13cb49 100644
> --- a/drivers/staging/bcm/DDRInit.c
> +++ b/drivers/staging/bcm/DDRInit.c
> @@ -1308,11 +1308,11 @@ int download_ddr_settings(struct bcm_mini_adapter
> *Adapter)
> if (!retval) {
> if (bOverrideSelfRefresh && (psDDRSetting->ulRegAddress
> == 0x0F007018)) {
> value = (psDDRSetting->ulRegValue | (1<<8));
> - if (STATUS_SUCCESS != wrmalt(Adapter,
> ul_ddr_setting_load_addr,
> - &value, sizeof(value))) {
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> "%s:%d\n", __func__, __LINE__);
> - break;
> - }
> + if (STATUS_SUCCESS != wrmalt(Adapter,
> ul_ddr_setting_load_addr,
> + &value, sizeof(value))) {
> + BCM_DEBUG_PRINT(Adapter,
> DBG_TYPE_PRINTK, 0, 0, "%s:%d\n", __func__, __LINE__);
> + break;
> + }
> } else {
> value = psDDRSetting->ulRegValue;
>
> --
> 2.0.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] Staging: bcm: Lines shortened in download_ddr_settings()
On Mon, Jun 23, 2014 at 11:42:33AM +0200, Matthias Beyer wrote:
> Signed-off-by: Matthias Beyer
As with the previous line-wrapping patch, this doesn't seem like a net
improvement; the lines seem far more readable un-wrapped.
> drivers/staging/bcm/DDRInit.c | 31 +--
> 1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/bcm/DDRInit.c b/drivers/staging/bcm/DDRInit.c
> index 423bfd9..4564f40 100644
> --- a/drivers/staging/bcm/DDRInit.c
> +++ b/drivers/staging/bcm/DDRInit.c
> @@ -1159,7 +1159,8 @@ int download_ddr_settings(struct bcm_mini_adapter
> *Adapter)
> {
> struct bcm_ddr_setting *psDDRSetting = NULL;
> ULONG RegCount = 0;
> - unsigned long ul_ddr_setting_load_addr =
> DDR_DUMP_INTERNAL_DEVICE_MEMORY;
> + unsigned long ul_ddr_setting_load_addr =
> + DDR_DUMP_INTERNAL_DEVICE_MEMORY;
> UINT value = 0;
> int retval = STATUS_SUCCESS;
> bool bOverrideSelfRefresh = false;
> @@ -1283,18 +1284,22 @@ int download_ddr_settings(struct bcm_mini_adapter
> *Adapter)
> }
> /* total number of Register that has to be dumped */
> value = RegCount;
> - retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> sizeof(value));
> + retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> + sizeof(value));
> if (retval) {
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "%s:%d\n",
> __func__, __LINE__);
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> + "%s:%d\n", __func__, __LINE__);
>
> return retval;
> }
> ul_ddr_setting_load_addr += sizeof(ULONG);
> /* signature */
> value = (0x1d1e0dd0);
> - retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> sizeof(value));
> + retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> + sizeof(value));
> if (retval) {
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "%s:%d\n",
> __func__, __LINE__);
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> + "%s:%d\n", __func__, __LINE__);
> return retval;
> }
>
> @@ -1303,17 +1308,23 @@ int download_ddr_settings(struct bcm_mini_adapter
> *Adapter)
>
> while (RegCount && !retval) {
> value = psDDRSetting->ulRegAddress;
> - retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> sizeof(value));
> + retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> + sizeof(value));
> ul_ddr_setting_load_addr += sizeof(ULONG);
> if (!retval) {
> - if (bOverrideSelfRefresh && (psDDRSetting->ulRegAddress
> == 0x0F007018))
> + if (bOverrideSelfRefresh
> + && (psDDRSetting->ulRegAddress
> + == 0x0F007018))
> value = (psDDRSetting->ulRegValue | (1<<8));
> else
> value = psDDRSetting->ulRegValue;
>
> - if (STATUS_SUCCESS != wrmalt(Adapter,
> ul_ddr_setting_load_addr,
> - &value, sizeof(value))) {
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> "%s:%d\n", __func__, __LINE__);
> + if (STATUS_SUCCESS != wrmalt(Adapter,
> + ul_ddr_setting_load_addr,
> + &value,
> + sizeof(value))) {
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> + "%s:%d\n", __func__, __LINE__);
> break;
> }
> }
> --
> 2.0.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] Staging: bcm: Lines shortened in download_ddr_settings()
On Wed, Jun 25, 2014 at 04:50:31PM +0200, Matthias Beyer wrote:
> On 23-06-2014 13:44:22, j...@joshtriplett.org wrote:
> > On Mon, Jun 23, 2014 at 11:42:33AM +0200, Matthias Beyer wrote:
> > > Signed-off-by: Matthias Beyer
> >
> > As with the previous line-wrapping patch, this doesn't seem like a net
> > improvement; the lines seem far more readable un-wrapped.
>
> I don't know. Previous patches which were similar were added.
> Readability is really much opinion-based.
>
> So should I rebase this patchset and remove the line-length patches?
>
> Are there more opinions on this?
I'm not the arbiter of kernel formatting; it's up to Greg (as the
maintainer of the staging tree) if he accepts the patches or not.
However, I've submitted a patch to checkpatch that would silence this
warning by default when not running with --strict, which should
de-emphasize it in favor of other, more objectively clear cleanups.
> > > drivers/staging/bcm/DDRInit.c | 31 +--
> > > 1 file changed, 21 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/staging/bcm/DDRInit.c b/drivers/staging/bcm/DDRInit.c
> > > index 423bfd9..4564f40 100644
> > > --- a/drivers/staging/bcm/DDRInit.c
> > > +++ b/drivers/staging/bcm/DDRInit.c
> > > @@ -1159,7 +1159,8 @@ int download_ddr_settings(struct bcm_mini_adapter
> > > *Adapter)
> > > {
> > > struct bcm_ddr_setting *psDDRSetting = NULL;
> > > ULONG RegCount = 0;
> > > - unsigned long ul_ddr_setting_load_addr =
> > > DDR_DUMP_INTERNAL_DEVICE_MEMORY;
> > > + unsigned long ul_ddr_setting_load_addr =
> > > + DDR_DUMP_INTERNAL_DEVICE_MEMORY;
> > > UINT value = 0;
> > > int retval = STATUS_SUCCESS;
> > > bool bOverrideSelfRefresh = false;
> > > @@ -1283,18 +1284,22 @@ int download_ddr_settings(struct bcm_mini_adapter
> > > *Adapter)
> > > }
> > > /* total number of Register that has to be dumped */
> > > value = RegCount;
> > > - retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> > > sizeof(value));
> > > + retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> > > + sizeof(value));
> > > if (retval) {
> > > - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "%s:%d\n",
> > > __func__, __LINE__);
> > > + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> > > + "%s:%d\n", __func__, __LINE__);
> > >
> > > return retval;
> > > }
> > > ul_ddr_setting_load_addr += sizeof(ULONG);
> > > /* signature */
> > > value = (0x1d1e0dd0);
> > > - retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> > > sizeof(value));
> > > + retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> > > + sizeof(value));
> > > if (retval) {
> > > - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "%s:%d\n",
> > > __func__, __LINE__);
> > > + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> > > + "%s:%d\n", __func__, __LINE__);
> > > return retval;
> > > }
> > >
> > > @@ -1303,17 +1308,23 @@ int download_ddr_settings(struct bcm_mini_adapter
> > > *Adapter)
> > >
> > > while (RegCount && !retval) {
> > > value = psDDRSetting->ulRegAddress;
> > > - retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> > > sizeof(value));
> > > + retval = wrmalt(Adapter, ul_ddr_setting_load_addr, &value,
> > > + sizeof(value));
> > > ul_ddr_setting_load_addr += sizeof(ULONG);
> > > if (!retval) {
> > > - if (bOverrideSelfRefresh && (psDDRSetting->ulRegAddress
> > > == 0x0F007018))
> > > + if (bOverrideSelfRefresh
> > > + && (psDDRSetting->ulRegAddress
> > > + == 0x0F007018))
> > > value = (psDDRSetting->ulRegValue | (1<<8));
> > > else
> > > value = psDDRSetting->ulRegValue;
> > >
> > > - if (STATUS_SUCCESS != wrmalt(Adapter,
> > > ul_ddr_setting_load_addr,
> > > - &value, sizeof(value))) {
> > > - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> > > "%s:%d\n", __func__, __LINE__);
> > > + if (STATUS_SUCCESS != wrmalt(Adapter,
> > > + ul_ddr_setting_load_addr,
> > > + &value,
> > > + sizeof(value))) {
> > > + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
> > > + "%s:%d\n", __func__, __LINE__);
> > > break;
> > > }
> > > }
> > > --
> > > 2.0.0
> > >
>
> --
> Mit freundlichen Grüßen,
> Kind regards,
> Matthias Beyer
>
> Proudly sent with mutt.
> Happily signed with gnupg.
__
Re: [PATCH] staging: pi433: #define shift constants in rf69.c
On Wed, Nov 08, 2017 at 02:52:30PM +0300, Dan Carpenter wrote:
> On Wed, Nov 08, 2017 at 06:25:06AM -0500, Joshua Abraham wrote:
> > This patch completes TODO improvements in rf69.c to change shift
> > constants to a define.
> >
> > Signed-off-by: Joshua Abraham
> > ---
> > drivers/staging/pi433/rf69.c | 4 ++--
> > drivers/staging/pi433/rf69_registers.h | 4
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> > index e69a2153c999..cfcace195be9 100644
> > --- a/drivers/staging/pi433/rf69.c
> > +++ b/drivers/staging/pi433/rf69.c
> > @@ -102,7 +102,7 @@ enum modulation rf69_get_modulation(struct spi_device
> > *spi)
> >
> > currentValue = READ_REG(REG_DATAMODUL);
> >
> > - switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO
> > improvement: change 3 to define
> > + switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >>
> > SHIFT_DATAMODUL_MODE) {
>
> You've send a few of mechanical patches without waiting for feedback and
> you should probably slow down...
>
Understood. I am just excited about submitting patches.
> The first thing to notice is that the original code is probably buggy
> and needs parenthesis.
>
> switch ((currentValue & MASK_DATAMODUL_MODULATION_TYPE) >> 3) {
>
> But that still doesn't fix the problem that x18 >> 3 is never going to
> equal to DATAMODUL_MODULATION_TYPE_OOK which is 0x8... So there are a
> couple bugs here.
>
> The line is over 80 characters, so checkpatch will complain about your
> patch. Please run checkpatch.pl on all your patches. Really, I hate
> all the naming here... Surely we can think of a better name than
> MASK_DATAMODUL_MODULATION_TYPE? Normally the "MASK" and "SHIFT" part of
> the name go at the end instead of the start.
>
I named the define to be consistent with the extant code, but I agree
that the names could be better.
> > /* RegDataModul */
> > +#define SHIFT_DATAMODUL_MODE 0x03
> > +
> > #define MASK_DATAMODUL_MODE 0x06
>
> Why did you add a blank line? Don't use hex values for shifting, use
> normal numbers. The 0x3 is indented too far.
>
I added the blank line to separate shifts from masks, but since the shift
will only be performed on the mask I supposed it isn't needed.
> Anyway, take your time and really think about patches before you send
> them. Normally, I write a patch, then wait overnight, then review it
> and again in the morning before I send it.
>
> regards,
> dan carpenter
>
Thanks for the criticism. I will be better.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: fsl-dpaa2: Fix multiple assignments should be avoided
On Wed, Nov 08, 2017 at 10:20:48AM +0100, Greg KH wrote: > On Tue, Nov 07, 2017 at 07:45:03PM -0500, Joshua Abraham wrote: > > This patch fixes the checkpatch.pl warning: > > "CHECK: multiple assignments should be avoided" > > > > Signed-off-by: Joshua Abraham > > --- > > drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c > > b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c > > index 0d8ed002adcb..384218946108 100644 > > --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c > > +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c > > @@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv > > *priv) > > * This may well change at runtime, either through irqbalance or > > * through direct user intervention. > > */ > > - rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask); > > + rx_cpu = cpumask_first(&priv->dpio_cpumask); > > + txc_cpu = rx_cpu; > > The original code here makes much more sense, doesn't it? > > Sometimes checkpatch is wrong :) > > thanks, > > greg k-h It does make a lot more sense. I will trust checkpatch less, and my eyes more :) -Josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: nvec: Fix usleep_range is preferred over udelay
On Wed, Nov 29, 2017 at 06:07:53PM +0200, Mikko Perttunen wrote:
> On 11/29/2017 06:00 PM, Joshua Abraham wrote:
> > Signed-off-by: Joshua Abraham
> >
> > This patch fixes the issue:
> >
> > CHECK: usleep_range is preferred over udelay; see
> > Documentation/timers/timers-howto.txt
> >
> > ---
> > drivers/staging/nvec/nvec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> > index 4ff8f47385da..2a01ef4b54ff 100644
> > --- a/drivers/staging/nvec/nvec.c
> > +++ b/drivers/staging/nvec/nvec.c
> > @@ -631,7 +631,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
> > break;
> > case 2: /* first byte after command */
> > if (status == (I2C_SL_IRQ | RNW | RCVD)) {
> > - udelay(33);
> > + usleep_range(30, 35);
> > if (nvec->rx->data[0] != 0x01) {
> > dev_err(nvec->dev,
> > "Read without prior read command\n");
> >
>
> This is incorrect, as this function is an interrupt handler and we cannot
> sleep in interrupt context.
>
> Cheers,
> Mikko
My mistake. Thank you for the feedback!
-Josh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: xgifb: remove unused macro XGIPART3
On Thu, Nov 30, 2017 at 08:55:44AM +0300, Dan Carpenter wrote: > On Wed, Nov 29, 2017 at 09:53:48PM -0500, Joshua Abraham wrote: > > Signed-off-by: Joshua Abraham > > > > This patch removes the unused macro XGIPART3. > > > > The Signed-off-by line goes after the changelog. > > > --- > > drivers/staging/xgifb/XGI_main.h | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/staging/xgifb/XGI_main.h > > b/drivers/staging/xgifb/XGI_main.h > > index a3af1cbbf8ee..5f55d0a39bc1 100644 > > --- a/drivers/staging/xgifb/XGI_main.h > > +++ b/drivers/staging/xgifb/XGI_main.h > > @@ -25,7 +25,6 @@ MODULE_DEVICE_TABLE(pci, xgifb_pci_table); > > #define XGIDACD (xgifb_info->dev_info.P3c9) > > #define XGIPART1 (xgifb_info->dev_info.Part1Port) > > #define XGIPART2 (xgifb_info->dev_info.Part2Port) > > -#define XGIPART3 (xgifb_info->dev_info.Part3Port) > > That define isn't hurting anyone. > > > #define XGIPART4 (xgifb_info->dev_info.Part4Port) > > #define XGIPART5 (xgifb_info->dev_info.Part5Port) > > Actually these should all be deleted because they mean you have to have > a xgifb_info variable and they hurt readability by hiding stuff behind a > define. It would be better to remove them all than to just remove one > from the middle. That's a more complicated patch, but it's a useful > patch. > > regards, > dan carpenter > Great point. I will work on that and get the patch out! -Josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: xgifb: remove macros with hidden variable
On Fri, Dec 01, 2017 at 10:12:30AM +0300, Dan Carpenter wrote:
> On Thu, Nov 30, 2017 at 10:39:48AM -0500, Joshua Abraham wrote:
> > diff --git a/drivers/staging/xgifb/XGI_main_26.c
> > b/drivers/staging/xgifb/XGI_main_26.c
> > index 6feecc55d2bc..6de66eaad96b 100644
> > --- a/drivers/staging/xgifb/XGI_main_26.c
> > +++ b/drivers/staging/xgifb/XGI_main_26.c
> > @@ -34,16 +34,16 @@ static void dumpVGAReg(struct xgifb_video_info
> > *xgifb_info)
> > {
> > u8 i, reg;
> >
> > - xgifb_reg_set(XGISR, 0x05, 0x86);
> > + xgifb_reg_set(xgifb_info->dev_info.P3c4, 0x05, 0x86);
>
> This patch is OK, but it might be nicer to create a temporary variable
> so the lines are not so long:
>
> struct vb_device_info *vb = &xgifb_info->dev_info;
> u8 i, reg;
>
> xgifb_reg_set(vb->P3c4, 0x05, 0x86);
>
> I chose "vb" based on the struct name... "dev" and "info" aren't very
> useful in a name because there are a lot of devices and lots of types
> of info.
>
> regards,
> dan carpneter
>
That is indeed MUCH more readable. Thanks, I'll work on this today.
-Josh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: davinci_vpfe: fix error check
On Sat, Feb 15, 2014 at 11:17:11AM +0100, Levente Kurusa wrote:
> The check would check the pointer, which is never less than 0.
> According to the error message, the correct check would be
> to check the return value of ipipe_mode. Check that instead.
>
> Reported-by: David Binderman
> Signed-off-by: Levente Kurusa
Reviewed-by: Josh Triplett
> drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> index 2d36b60..b2daf5e 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> @@ -267,7 +267,7 @@ int config_ipipe_hw(struct vpfe_ipipe_device *ipipe)
> }
>
> ipipe_mode = get_ipipe_mode(ipipe);
> - if (ipipe < 0) {
> + if (ipipe_mode < 0) {
> pr_err("Failed to get ipipe mode");
> return -EINVAL;
> }
> --
> 1.8.3.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: cxt1e1: hwprobe Fix different address spaces
On Fri, Mar 21, 2014 at 03:52:23PM +0200, Matei Oprea wrote:
> Fix different address spaces when unmapping IO. hi->addr_mapped[0]
> and hi->addr_mapped[1] should be tagged __iomem.
>
> Signed-off-by: Matei Oprea
> Cc: ROSEdu Kernel Community
> ---
> drivers/staging/cxt1e1/hwprobe.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/cxt1e1/hwprobe.c
> b/drivers/staging/cxt1e1/hwprobe.c
> index 9b4198b..f55c27e 100644
> --- a/drivers/staging/cxt1e1/hwprobe.c
> +++ b/drivers/staging/cxt1e1/hwprobe.c
> @@ -174,12 +174,12 @@ cleanup_ioremap(void)
> if (hi->pci_slot == 0xff)
> break;
> if (hi->addr_mapped[0]) {
> - iounmap((void *)(hi->addr_mapped[0]));
> + iounmap((void __iomem *)(hi->addr_mapped[0]));
While this may "fix" the sparse warning, it's far from ideal. Like I
had suggested on IRC, a much better cleanup would be to make the
addr_mapped member an array of __iomem-tagged pointers, and fixup the
users.
Josh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8187se: Fix warning symbol should be static
On Sun, Apr 06, 2014 at 05:46:04PM +0200, Jonas Hahnfeld wrote:
> This patch solves some sparse warnings about "symbol [...] was not
> declared. Should it be static?" by including the correct header files.
>
> Signed-off-by: Jonas Hahnfeld
Reviewed-by: Josh Triplett
> Tested by compile only
>
> drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.c | 1 +
> drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_ccmp.c | 1 +
> drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_wep.c | 1 +
> drivers/staging/rtl8187se/r8180_wx.c | 1 +
> 4 files changed, 4 insertions(+)
>
> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.c
> b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.c
> index 101f0c0..03462c3 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.c
> @@ -19,6 +19,7 @@
> #include
> #include
>
> +#include "../r8180.h"
> #include "ieee80211.h"
>
> MODULE_AUTHOR("Jouni Malinen");
> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_ccmp.c
> b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_ccmp.c
> index 4fe2538..7eae3d9 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_ccmp.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_ccmp.c
> @@ -21,6 +21,7 @@
> #include
> #include
>
> +#include "../r8180.h"
> #include "ieee80211.h"
>
> #include
> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_wep.c
> b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_wep.c
> index f253672..288f86c 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_wep.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt_wep.c
> @@ -17,6 +17,7 @@
> #include
> #include
>
> +#include "../r8180.h"
> #include "ieee80211.h"
>
> #include
> diff --git a/drivers/staging/rtl8187se/r8180_wx.c
> b/drivers/staging/rtl8187se/r8180_wx.c
> index b552491..57bf01f 100644
> --- a/drivers/staging/rtl8187se/r8180_wx.c
> +++ b/drivers/staging/rtl8187se/r8180_wx.c
> @@ -20,6 +20,7 @@
>
> #include "r8180.h"
> #include "r8180_hw.h"
> +#include "r8180_wx.h"
>
> #include
> #include "ieee80211/dot11d.h"
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8187se: fix pointer and return statement's syntax
On Wed, Apr 09, 2014 at 09:25:55AM +0200, Martin Kepplinger wrote:
> Use the common kernel coding style.
>
> Signed-off-by: Martin Kepplinger
Reviewed-by: Josh Triplett
>
> drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> index 0dc5ae4..e6257b3 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> @@ -180,7 +180,7 @@ static inline int ieee80211_put_snap(u8 *data, u16
> h_proto)
> int ieee80211_encrypt_fragment(struct ieee80211_device *ieee,
> struct sk_buff *frag, int hdr_len)
> {
> - struct ieee80211_crypt_data* crypt = ieee->crypt[ieee->tx_keyidx];
> + struct ieee80211_crypt_data *crypt = ieee->crypt[ieee->tx_keyidx];
> int res;
>
> /*
> @@ -285,7 +285,7 @@ static int ieee80211_classify(struct sk_buff *skb,
>
> if (!network->QoS_Enable) {
> skb->priority = 0;
> - return(wme_UP);
> + return wme_UP;
> }
>
> if (eh->ether_type == __constant_htons(ETHERTYPE_IP)) {
> @@ -304,7 +304,7 @@ static int ieee80211_classify(struct sk_buff *skb,
> }
>
> skb->priority = wme_UP;
> - return(wme_UP);
> + return wme_UP;
> }
>
> /* SKBs are added to the ieee->tx_queue. */
> --
> 1.7.10.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wlan-ng: fix bad symbol declaration
On Fri, Apr 11, 2014 at 03:00:49PM +0200, Neil Armstrong wrote:
> With sparse, the following error appears :
> CHECK drivers/staging/wlan-ng/p80211netdev.c
> drivers/staging/wlan-ng/cfg80211.c:710:6: warning: symbol
> 'prism2_connect_result' was not declared. Should it be static?
> drivers/staging/wlan-ng/cfg80211.c:719:6: warning: symbol
> 'prism2_disconnected' was not declared. Should it be static?
> drivers/staging/wlan-ng/cfg80211.c:725:6: warning: symbol 'prism2_roamed' was
> not declared. Should it be static?
>
> Move functions declaration to coherent internal header file.
>
> Signed-off-by: Neil 'Superna' Armstrong
Reviewed-by: Josh Triplett
> drivers/staging/wlan-ng/prism2mgmt.h |5 +
> drivers/staging/wlan-ng/prism2sta.c |4
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/wlan-ng/prism2mgmt.h
> b/drivers/staging/wlan-ng/prism2mgmt.h
> index 190d390..b62fdcb 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.h
> +++ b/drivers/staging/wlan-ng/prism2mgmt.h
> @@ -109,4 +109,9 @@ void prism2sta_processing_defer(struct work_struct *data);
> void prism2sta_commsqual_defer(struct work_struct *data);
> void prism2sta_commsqual_timer(unsigned long data);
>
> +/* Interface callback functions, passing data back up to the cfg80211 layer
> */
> +void prism2_connect_result(wlandevice_t *wlandev, u8 failed);
> +void prism2_disconnected(wlandevice_t *wlandev);
> +void prism2_roamed(wlandevice_t *wlandev);
> +
> #endif
> diff --git a/drivers/staging/wlan-ng/prism2sta.c
> b/drivers/staging/wlan-ng/prism2sta.c
> index f9ccf23..3fd4538 100644
> --- a/drivers/staging/wlan-ng/prism2sta.c
> +++ b/drivers/staging/wlan-ng/prism2sta.c
> @@ -120,10 +120,6 @@ MODULE_PARM_DESC(prism2_reset_settletime, "reset settle
> time in ms");
>
> MODULE_LICENSE("Dual MPL/GPL");
>
> -void prism2_connect_result(wlandevice_t *wlandev, u8 failed);
> -void prism2_disconnected(wlandevice_t *wlandev);
> -void prism2_roamed(wlandevice_t *wlandev);
> -
> static int prism2sta_open(wlandevice_t *wlandev);
> static int prism2sta_close(wlandevice_t *wlandev);
> static void prism2sta_reset(wlandevice_t *wlandev);
> --
> 1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [OPW kernel] Re: [PATCH v2] Staging: zram: Fix variable dereferenced before check
On Sat, Oct 19, 2013 at 01:59:05PM -0700, Greg KH wrote: > On Sat, Oct 19, 2013 at 10:01:42PM +0530, Rashika Kheria wrote: > > This patch fixes the following Smatch warning in zram_drv.c- > > ~/git/kernels/linux/drivers/staging/zram/zram_drv.c:663 > > reset_store() warn: variable dereferenced before check 'bdev' (see line 652) > > ~/git/kernels/linux/drivers/staging/zram/zram_drv.c:899 > > destroy_device() warn: variable dereferenced before check 'zram->disk' (see > > line 896) > > > > Signed-off-by: Rashika Kheria > > zram is messy, tricky, and I hate it. Seriously, I which I had never > taken it into the staging tree... > > Anyway, I want the existing zram developers/maintainers to review this > patch before I can accept it, as I don't trust anything that is ever > done in that code, especially as I don't test it :) > > Minchan, Jiang, Nitin, what do you think of the patch below? Can I get > your ack on it so that I can apply it? I think you actually want to review v3 of this patch, not v2. - Josh Triplett ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] Staging: sb105x: info leak in mp_get_count()
On Tue, Oct 29, 2013 at 11:01:43PM +0300, Dan Carpenter wrote:
> The icount.reserved[] array isn't initialized so it leaks stack
> information to userspace.
>
> Reported-by: Nico Golde
> Reported-by: Fabian Yamaguchi
> Signed-off-by: Dan Carpenter
Reviewed-by: Josh Triplett
Also, you don't quite have the patch format right here; you should have
a --- line after the commit mesage, followed by a diffstat. Did you use
git format-patch to generate this patch?
> diff --git a/drivers/staging/sb105x/sb_pci_mp.c
> b/drivers/staging/sb105x/sb_pci_mp.c
> index bc53b4e..bde28b9 100644
> --- a/drivers/staging/sb105x/sb_pci_mp.c
> +++ b/drivers/staging/sb105x/sb_pci_mp.c
> @@ -1063,7 +1063,7 @@ static int mp_wait_modem_status(struct sb_uart_state
> *state, unsigned long arg)
>
> static int mp_get_count(struct sb_uart_state *state, struct
> serial_icounter_struct *icnt)
> {
> - struct serial_icounter_struct icount;
> + struct serial_icounter_struct icount = {};
> struct sb_uart_icount cnow;
> struct sb_uart_port *port = state->port;
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] Staging: sb105x: info leak in mp_get_count()
On Mon, Nov 04, 2013 at 02:11:50AM +0300, Dan Carpenter wrote: > On Sun, Nov 03, 2013 at 10:28:02AM -0800, Josh Triplett wrote: > > On Tue, Oct 29, 2013 at 11:01:43PM +0300, Dan Carpenter wrote: > > > The icount.reserved[] array isn't initialized so it leaks stack > > > information to userspace. > > > > > > Reported-by: Nico Golde > > > Reported-by: Fabian Yamaguchi > > > Signed-off-by: Dan Carpenter > > > > Reviewed-by: Josh Triplett > > > > Also, you don't quite have the patch format right here; you should have > > a --- line after the commit mesage, followed by a diffstat. Did you use > > git format-patch to generate this patch? > > I normally don't include the diffstat. Which tools care about that? Human wetware. :) It isn't required by any tools. The --- is, though, to produce something applicable by git. - Josh Triplett ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] Staging: sb105x: info leak in mp_get_count()
On Mon, Nov 04, 2013 at 10:01:00AM +0300, Dan Carpenter wrote: > I've dropped most of the people from the CC list. > > On Sun, Nov 03, 2013 at 08:31:50PM -0800, Josh Triplett wrote: > > On Mon, Nov 04, 2013 at 02:11:50AM +0300, Dan Carpenter wrote: > > > On Sun, Nov 03, 2013 at 10:28:02AM -0800, Josh Triplett wrote: > > > > On Tue, Oct 29, 2013 at 11:01:43PM +0300, Dan Carpenter wrote: > > > > > The icount.reserved[] array isn't initialized so it leaks stack > > > > > information to userspace. > > > > > > > > > > Reported-by: Nico Golde > > > > > Reported-by: Fabian Yamaguchi > > > > > Signed-off-by: Dan Carpenter > > > > > > > > Reviewed-by: Josh Triplett > > > > > > > > Also, you don't quite have the patch format right here; you should have > > > > a --- line after the commit mesage, followed by a diffstat. Did you use > > > > git format-patch to generate this patch? > > > > > > I normally don't include the diffstat. Which tools care about that? > > > > Human wetware. :) > > > > It isn't required by any tools. The --- is, though, to produce > > something applicable by git. > > That's really weird. I've been using the same scripts for years and no > one has complained before. The patch applies fine with `git am` for me. > I'm using git version 1.7.10.4. I stand corrected. I was under the impression that the --- was required to mark the end of the commit message, but sure enough, git am seems to accept it. Reading the git am manpage, it says that a line starting with "diff -" will also indicate the end of the commit message and start of the patch. It still isn't the conventional format produced by git format-patch, which I'd recommend matching for ease of human consumption, but nonetheless it apparently works. - Josh Triplett ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
imx-drm/ipuv3-crtc.ko fails to link
Hi All, With v3.12-2839-gedae583 (Linus' tree as of this morning), the ipuv3-crtc.ko module fails to link with the following messages: ERROR: "ipu_plane_disable" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "ipu_plane_enable" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "ipu_plane_irq" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "ipu_plane_get_resources" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "ipu_plane_init" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "imx_drm_crtc_id" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "ipu_plane_mode_set" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "ipu_plane_set_base" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! ERROR: "ipu_plane_put_resources" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! make[1]: *** [__modpost] Error 1 make: *** [modules] Error 2 The relevant .config hunks are configured as such: CONFIG_DRM_IMX=m CONFIG_DRM_IMX_FB_HELPER=m CONFIG_DRM_IMX_PARALLEL_DISPLAY=m CONFIG_DRM_IMX_TVE=m # CONFIG_DRM_IMX_LDB is not set CONFIG_DRM_IMX_IPUV3_CORE=m CONFIG_DRM_IMX_IPUV3=m I can send the full .config if people would like to see it. I'm guessing the ipu_plane_* functions listed here need EXPORT_SYMBOL statements added to them. Is this currently queued in anyone's tree? josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: imx-drm/ipuv3-crtc.ko fails to link
On Mon, Nov 11, 2013 at 11:22 AM, Josh Boyer wrote: > Hi All, > > With v3.12-2839-gedae583 (Linus' tree as of this morning), the > ipuv3-crtc.ko module fails to link with the following messages: > > ERROR: "ipu_plane_disable" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "ipu_plane_enable" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "ipu_plane_irq" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "ipu_plane_get_resources" > [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "ipu_plane_init" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "imx_drm_crtc_id" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "ipu_plane_mode_set" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "ipu_plane_set_base" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > ERROR: "ipu_plane_put_resources" > [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! > make[1]: *** [__modpost] Error 1 > make: *** [modules] Error 2 Actually, I think this is because ipuv3-plane.c was added to the Makefile as a requisite file for ipuv3-crtc.ko with commit b8d181e408af6a017d, but Kbuild isn't interpreting it that way because it's a tristate option, not a bool. Making it work would require something like: imx-ipuv3-crct-objs := ipuv3-crtc.o ipuv3-plane.o obj-$(CONFIG_DRM_IMX_IPUV3) += imx-ipuv3-crct.o But that would rename the module from ipuv3-crtc.ko to imx-ipuv3-crct.ko. Even with that, it still fails to link with: ERROR: "imx_drm_crtc_id" [drivers/staging/imx-drm/imx-ipuv3-crtc.ko] undefined! So two questions really. 1) Are the tristate options for DRM_IMX_IPUV3, DRM_IMX_IPUV3_CORE, DRM_IMX_LDB, DRM_IMX_TVE, DRM_IMX_PARALLEL_DISPLAY, and DRM_IMX_FB_HELPER really all supposed to be separate modules, or should they be boolean options to the main DRM_IMX tristate? 2) If the answer to question 1 is "all separate modules", then either ipuv3-crtc.c needs to be renamed or the final module name will be different. And did anyone actually try building all of these as modules? This used to work in 3.12, but I'm not sure that was actually tested there either. josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: imx-drm/ipuv3-crtc.ko fails to link
On Tue, Nov 12, 2013 at 9:57 AM, Sascha Hauer wrote: > Hi Josh, > > On Tue, Nov 12, 2013 at 09:32:08AM -0500, Josh Boyer wrote: >> On Mon, Nov 11, 2013 at 11:22 AM, Josh Boyer >> wrote: >> > Hi All, >> > >> > With v3.12-2839-gedae583 (Linus' tree as of this morning), the >> > ipuv3-crtc.ko module fails to link with the following messages: >> > >> > ERROR: "ipu_plane_disable" [drivers/staging/imx-drm/ipuv3-crtc.ko] >> > undefined! >> > ERROR: "ipu_plane_enable" [drivers/staging/imx-drm/ipuv3-crtc.ko] >> > undefined! >> > ERROR: "ipu_plane_irq" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! >> > ERROR: "ipu_plane_get_resources" >> > [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! >> > ERROR: "ipu_plane_init" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! >> > ERROR: "imx_drm_crtc_id" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! >> > ERROR: "ipu_plane_mode_set" [drivers/staging/imx-drm/ipuv3-crtc.ko] >> > undefined! >> > ERROR: "ipu_plane_set_base" [drivers/staging/imx-drm/ipuv3-crtc.ko] >> > undefined! >> > ERROR: "ipu_plane_put_resources" >> > [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined! >> > make[1]: *** [__modpost] Error 1 >> > make: *** [modules] Error 2 >> >> Actually, I think this is because ipuv3-plane.c was added to the >> Makefile as a requisite file for ipuv3-crtc.ko with commit >> b8d181e408af6a017d, but Kbuild isn't interpreting it that way because >> it's a tristate option, not a bool. Making it work would require >> something like: >> >> imx-ipuv3-crct-objs := ipuv3-crtc.o ipuv3-plane.o >> obj-$(CONFIG_DRM_IMX_IPUV3) += imx-ipuv3-crct.o >> >> But that would rename the module from ipuv3-crtc.ko to >> imx-ipuv3-crct.ko. Even with that, it still fails to link with: >> >> ERROR: "imx_drm_crtc_id" [drivers/staging/imx-drm/imx-ipuv3-crtc.ko] >> undefined! >> >> So two questions really. >> >> 1) Are the tristate options for DRM_IMX_IPUV3, DRM_IMX_IPUV3_CORE, >> DRM_IMX_LDB, DRM_IMX_TVE, DRM_IMX_PARALLEL_DISPLAY, and >> DRM_IMX_FB_HELPER really all supposed to be separate modules, or >> should they be boolean options to the main DRM_IMX tristate? > > I think at least the ipuv3 core should be a separate module since this > could be used without drm support (for the input pathes, handled via v4l2) > >> >> 2) If the answer to question 1 is "all separate modules", then either >> ipuv3-crtc.c needs to be renamed or the final module name will be >> different. > > The final module name shouldn't matter, we can change it. OK. > Does a EXPORT_SYMBOL_GPL(imx_drm_crtc_id) help fixing the problem? I'm sure it would. Haven't had a chance to test it yet. Since the module name change isn't a big deal, I'll see if I can come up with a patch to fix the Makefile and linking issues and send that out. >> And did anyone actually try building all of these as modules? This >> used to work in 3.12, but I'm not sure that was actually tested there >> either. > > At least I explicitly made this work when writing the IPU stuff, but I > must admit that we mostly use monolithic kernels here. I figured as much. Fedora builds them as modules, presumably because we build multiboard kernels and building this in doesn't benefit everything those run on. josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: imx-drm: Fix modular build of DRM_IMX_IPUV3
commit b8d181e408af (staging: drm/imx: add drm plane support) added a file
to the make target for DRM_IMX_IPUV3 but didn't adjust the objs required
to actually build that as a module. Kbuild got confused and this lead to
link errors like:
ERROR: "ipu_plane_disable" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined!
ERROR: "ipu_plane_enable" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined!
Additionally, it added a call to imx_drm_crtc_id which also fails with a
link error as above. To fix this, we adjust the make target with the proper
objs, which will change the name of the resulting .ko. We also add an
EXPORT_SYMBOL_GPL for imx_drm_crtc_id.
Signed-off-by: Josh Boyer
---
drivers/staging/imx-drm/Makefile | 4 +++-
drivers/staging/imx-drm/imx-drm-core.c | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile
index 2c3a9e1..8742432 100644
--- a/drivers/staging/imx-drm/Makefile
+++ b/drivers/staging/imx-drm/Makefile
@@ -8,4 +8,6 @@ obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
obj-$(CONFIG_DRM_IMX_FB_HELPER) += imx-fbdev.o
obj-$(CONFIG_DRM_IMX_IPUV3_CORE) += ipu-v3/
-obj-$(CONFIG_DRM_IMX_IPUV3)+= ipuv3-crtc.o ipuv3-plane.o
+
+imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o
+obj-$(CONFIG_DRM_IMX_IPUV3)+= imx-ipuv3-crtc.o
diff --git a/drivers/staging/imx-drm/imx-drm-core.c
b/drivers/staging/imx-drm/imx-drm-core.c
index 4483d47..2b366d8 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -72,6 +72,7 @@ int imx_drm_crtc_id(struct imx_drm_crtc *crtc)
{
return crtc->pipe;
}
+EXPORT_SYMBOL_GPL(imx_drm_crtc_id);
static void imx_drm_driver_lastclose(struct drm_device *drm)
{
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: imx-drm: Fix modular build of DRM_IMX_IPUV3
On Mon, Nov 18, 2013 at 10:03:10AM +0100, Sascha Hauer wrote:
> On Tue, Nov 12, 2013 at 12:15:45PM -0500, Josh Boyer wrote:
> > commit b8d181e408af (staging: drm/imx: add drm plane support) added a file
> > to the make target for DRM_IMX_IPUV3 but didn't adjust the objs required
> > to actually build that as a module. Kbuild got confused and this lead to
> > link errors like:
> >
> > ERROR: "ipu_plane_disable" [drivers/staging/imx-drm/ipuv3-crtc.ko]
> > undefined!
> > ERROR: "ipu_plane_enable" [drivers/staging/imx-drm/ipuv3-crtc.ko] undefined!
> >
> > Additionally, it added a call to imx_drm_crtc_id which also fails with a
> > link error as above. To fix this, we adjust the make target with the proper
> > objs, which will change the name of the resulting .ko. We also add an
> > EXPORT_SYMBOL_GPL for imx_drm_crtc_id.
> >
> > Signed-off-by: Josh Boyer
>
>
> Acked-by: Sascha Hauer
>
> Sascha
Thanks. So who picks this fix up? Greg, is that you?
josh
>
> > ---
> > drivers/staging/imx-drm/Makefile | 4 +++-
> > drivers/staging/imx-drm/imx-drm-core.c | 1 +
> > 2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/imx-drm/Makefile
> > b/drivers/staging/imx-drm/Makefile
> > index 2c3a9e1..8742432 100644
> > --- a/drivers/staging/imx-drm/Makefile
> > +++ b/drivers/staging/imx-drm/Makefile
> > @@ -8,4 +8,6 @@ obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
> > obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
> > obj-$(CONFIG_DRM_IMX_FB_HELPER) += imx-fbdev.o
> > obj-$(CONFIG_DRM_IMX_IPUV3_CORE) += ipu-v3/
> > -obj-$(CONFIG_DRM_IMX_IPUV3)+= ipuv3-crtc.o ipuv3-plane.o
> > +
> > +imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o
> > +obj-$(CONFIG_DRM_IMX_IPUV3)+= imx-ipuv3-crtc.o
> > diff --git a/drivers/staging/imx-drm/imx-drm-core.c
> > b/drivers/staging/imx-drm/imx-drm-core.c
> > index 4483d47..2b366d8 100644
> > --- a/drivers/staging/imx-drm/imx-drm-core.c
> > +++ b/drivers/staging/imx-drm/imx-drm-core.c
> > @@ -72,6 +72,7 @@ int imx_drm_crtc_id(struct imx_drm_crtc *crtc)
> > {
> > return crtc->pipe;
> > }
> > +EXPORT_SYMBOL_GPL(imx_drm_crtc_id);
> >
> > static void imx_drm_driver_lastclose(struct drm_device *drm)
> > {
> > --
> > 1.8.3.1
> >
> >
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: imx-drm: Fix modular build of DRM_IMX_IPUV3
On Tue, Nov 19, 2013 at 09:39:19PM -0800, Greg KH wrote: > On Tue, Nov 19, 2013 at 08:56:59PM -0500, Josh Boyer wrote: > > On Mon, Nov 18, 2013 at 10:03:10AM +0100, Sascha Hauer wrote: > > > On Tue, Nov 12, 2013 at 12:15:45PM -0500, Josh Boyer wrote: > > > > commit b8d181e408af (staging: drm/imx: add drm plane support) added a > > > > file > > > > to the make target for DRM_IMX_IPUV3 but didn't adjust the objs required > > > > to actually build that as a module. Kbuild got confused and this lead > > > > to > > > > link errors like: > > > > > > > > ERROR: "ipu_plane_disable" [drivers/staging/imx-drm/ipuv3-crtc.ko] > > > > undefined! > > > > ERROR: "ipu_plane_enable" [drivers/staging/imx-drm/ipuv3-crtc.ko] > > > > undefined! > > > > > > > > Additionally, it added a call to imx_drm_crtc_id which also fails with a > > > > link error as above. To fix this, we adjust the make target with the > > > > proper > > > > objs, which will change the name of the resulting .ko. We also add an > > > > EXPORT_SYMBOL_GPL for imx_drm_crtc_id. > > > > > > > > Signed-off-by: Josh Boyer > > > > > > > > > Acked-by: Sascha Hauer > > > > > > Sascha > > > > Thanks. So who picks this fix up? Greg, is that you? > > Yes, I will once 3.13-rc1 is out. OK. And send it to Linus for 3.13-rc2? I know you've been traveling and are busy, so I'm just pointing out it's a fix for 3.13. josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: ft1000: ft1000-usb: initialize 'status' with STATUS_SUCCESS in request_code_segment()
On Wed, Nov 27, 2013 at 11:01:18AM +0800, Chen Gang wrote:
> If "!bool_case", it returns unexpected value instead of STATUS_SUCCESS,
> so need fix it, the related warning (with allmodconfig under hexagon):
>
> CC [M] drivers/staging/ft1000/ft1000-usb/ft1000_download.o
> drivers/staging/ft1000/ft1000-usb/ft1000_download.c: In function
> 'request_code_segment':
> drivers/staging/ft1000/ft1000-usb/ft1000_download.c:581:6: warning:
> 'status' may be used uninitialized in this function [-Wuninitialized]
>
>
> Signed-off-by: Chen Gang
Reviewed-by: Josh Triplett
> .../staging/ft1000/ft1000-usb/ft1000_download.c|2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
> b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
> index 68ded17..15f3062 100644
> --- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
> +++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
> @@ -578,7 +578,7 @@ static int request_code_segment(struct ft1000_usb
> *ft1000dev, u16 **s_file,
>u8 **c_file, const u8 *endpoint, bool boot_case)
> {
> long word_length;
> - int status;
> + int status = STATUS_SUCCESS;
>
> /*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/
> word_length = get_request_value(ft1000dev);
> --
> 1.7.7.6
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: hv: Mark the function hv_synic_free_cpu() as static in hv.c
On Sat, Dec 14, 2013 at 07:00:06PM +0530, Rashika Kheria wrote:
> This patch marks the function hv_synic_free_cpu() as static in hv.c
> because it is not used outside this file.
>
> Thus, it also eliminates the following warning in hv.c:
> drivers/hv/hv.c:304:6: warning: no previous prototype for ‘hv_synic_free_cpu’
> [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
Reviewed-by: Josh Triplett
> drivers/hv/hv.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index f0c5e07..bcb4950 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -301,7 +301,7 @@ err:
> return -ENOMEM;
> }
>
> -void hv_synic_free_cpu(int cpu)
> +static void hv_synic_free_cpu(int cpu)
> {
> kfree(hv_context.event_dpc[cpu]);
> if (hv_context.synic_event_page[cpu])
> --
> 1.7.9.5
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: sm7xxfb: Mark function as static in sm7xxfb.c
On Sat, Dec 21, 2013 at 03:44:13PM +0530, Rashika Kheria wrote:
> Mark function smtcfb_setmode() as static in sm7xxfb.c because it is not
> used outside this file.
>
> This eliminates the following warning in sm7xxfb.c:
> drivers/staging/sm7xxfb/sm7xxfb.c:588:6: warning: no previous prototype for
> ‘smtcfb_setmode’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
Reviewed-by: Josh Triplett
> drivers/staging/sm7xxfb/sm7xxfb.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c
> b/drivers/staging/sm7xxfb/sm7xxfb.c
> index ba199ff..f08b216 100644
> --- a/drivers/staging/sm7xxfb/sm7xxfb.c
> +++ b/drivers/staging/sm7xxfb/sm7xxfb.c
> @@ -585,7 +585,7 @@ static void smtc_set_timing(struct smtcfb_info *sfb)
> }
> }
>
> -void smtcfb_setmode(struct smtcfb_info *sfb)
> +static void smtcfb_setmode(struct smtcfb_info *sfb)
> {
> switch (sfb->fb.var.bits_per_pixel) {
> case 32:
> --
> 1.7.9.5
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: dwc2: Mark function as static in core.c
On Sat, Dec 21, 2013 at 03:50:29PM +0530, Rashika Kheria wrote:
> Mark function dwc2_set_param_uframe_sched() as static in core.c because
> it is not used outside this file.
>
> This eliminates the following warning in core.c:
> drivers/staging/dwc2/core.c:2739:5: warning: no previous prototype for
> ‘dwc2_set_param_uframe_sched’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
Reviewed-by: Josh Triplett
> drivers/staging/dwc2/core.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c
> index 6d001b5..c029cb6 100644
> --- a/drivers/staging/dwc2/core.c
> +++ b/drivers/staging/dwc2/core.c
> @@ -2736,7 +2736,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
> return 0;
> }
>
> -int dwc2_set_param_uframe_sched(struct dwc2_hsotg *hsotg, int val)
> +static int dwc2_set_param_uframe_sched(struct dwc2_hsotg *hsotg, int val)
> {
> int retval = 0;
>
> --
> 1.7.9.5
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] drivers: dgap: Include appropriate header file in dgap_parse.c
On Sat, Dec 21, 2013 at 03:55:50PM +0530, Rashika Kheria wrote: > Include appropriate header file in dgap/dgap_parse.h in dgap_parse.c > because functions dgap_parsefile(), dgap_config_get_useintr(), > dgap_config_get_altpin(), dgap_find_config(), > dgap_config_get_number_of_ports(), dgap_create_config_string() and > dgap_get_config_letters() have their prototype declarations in the > header file. > > This eliminates the following warnings in dgap_parse.c: > drivers/staging/dgap/dgap_parse.c:125:5: warning: no previous prototype for > ‘dgap_parsefile’ [-Wmissing-prototypes] > drivers/staging/dgap/dgap_parse.c:1101:6: warning: no previous prototype for > ‘dgap_config_get_useintr’ [-Wmissing-prototypes] > drivers/staging/dgap/dgap_parse.c:1128:6: warning: no previous prototype for > ‘dgap_config_get_altpin’ [-Wmissing-prototypes] > drivers/staging/dgap/dgap_parse.c:1157:15: warning: no previous prototype for > ‘dgap_find_config’ [-Wmissing-prototypes] > drivers/staging/dgap/dgap_parse.c:1223:6: warning: no previous prototype for > ‘dgap_config_get_number_of_ports’ [-Wmissing-prototypes] > drivers/staging/dgap/dgap_parse.c:1252:7: warning: no previous prototype for > ‘dgap_create_config_string’ [-Wmissing-prototypes] > drivers/staging/dgap/dgap_parse.c:1311:7: warning: no previous prototype for > ‘dgap_get_config_letters’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/staging/dgap/dgap_parse.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/dgap/dgap_parse.c > b/drivers/staging/dgap/dgap_parse.c > index ff9d194..36fd93d 100644 > --- a/drivers/staging/dgap/dgap_parse.c > +++ b/drivers/staging/dgap/dgap_parse.c > @@ -42,6 +42,7 @@ > #include "dgap_types.h" > #include "dgap_fep5.h" > #include "dgap_driver.h" > +#include "dgap_parse.h" > #include "dgap_conf.h" > > > -- > 1.7.9.5 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: sbe-2t3e3: Mark functions as static in ctrl.c
On Sat, Dec 21, 2013 at 03:47:13PM +0530, Rashika Kheria wrote:
> Mark functions t3e3_set_loopback(), t3e3_reg_read(), t3e3_reg_write(),
> t3e3_port_get(), t3e3_port_set(), t3e3_port_get_stats() and
> t3e3_port_del_stats() as static in ctrl.c because they are not used
> outside this file.
>
> This eliminates the following warnings in ctrl.c:
> drivers/staging/sbe-2t3e3/ctrl.c:34:6: warning: no previous prototype for
> ‘t3e3_set_loopback’ [-Wmissing-prototypes]
> drivers/staging/sbe-2t3e3/ctrl.c:98:6: warning: no previous prototype for
> ‘t3e3_reg_read’ [-Wmissing-prototypes]
> drivers/staging/sbe-2t3e3/ctrl.c:135:6: warning: no previous prototype for
> ‘t3e3_reg_write’ [-Wmissing-prototypes]
> drivers/staging/sbe-2t3e3/ctrl.c:167:6: warning: no previous prototype for
> ‘t3e3_port_get’ [-Wmissing-prototypes]
> drivers/staging/sbe-2t3e3/ctrl.c:172:6: warning: no previous prototype for
> ‘t3e3_port_set’ [-Wmissing-prototypes]
> drivers/staging/sbe-2t3e3/ctrl.c:219:6: warning: no previous prototype for
> ‘t3e3_port_get_stats’ [-Wmissing-prototypes]
> drivers/staging/sbe-2t3e3/ctrl.c:285:6: warning: no previous prototype for
> ‘t3e3_port_del_stats’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
Reviewed-by: Josh Triplett
> drivers/staging/sbe-2t3e3/ctrl.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/sbe-2t3e3/ctrl.c
> b/drivers/staging/sbe-2t3e3/ctrl.c
> index a5825d7..d280bcf 100644
> --- a/drivers/staging/sbe-2t3e3/ctrl.c
> +++ b/drivers/staging/sbe-2t3e3/ctrl.c
> @@ -31,7 +31,7 @@ void t3e3_set_frame_type(struct channel *sc, u32 mode)
> sc->p.frame_type = mode;
> }
>
> -void t3e3_set_loopback(struct channel *sc, u32 mode)
> +static void t3e3_set_loopback(struct channel *sc, u32 mode)
> {
> u32 tx, rx;
>
> @@ -95,7 +95,7 @@ void t3e3_set_loopback(struct channel *sc, u32 mode)
> }
>
>
> -void t3e3_reg_read(struct channel *sc, u32 *reg, u32 *val)
> +static void t3e3_reg_read(struct channel *sc, u32 *reg, u32 *val)
> {
> u32 i;
>
> @@ -132,7 +132,7 @@ void t3e3_reg_read(struct channel *sc, u32 *reg, u32 *val)
> }
> }
>
> -void t3e3_reg_write(struct channel *sc, u32 *reg)
> +static void t3e3_reg_write(struct channel *sc, u32 *reg)
> {
> u32 i;
>
> @@ -164,12 +164,12 @@ void t3e3_reg_write(struct channel *sc, u32 *reg)
> }
> }
>
> -void t3e3_port_get(struct channel *sc, t3e3_param_t *param)
> +static void t3e3_port_get(struct channel *sc, t3e3_param_t *param)
> {
> memcpy(param, &(sc->p), sizeof(t3e3_param_t));
> }
>
> -void t3e3_port_set(struct channel *sc, t3e3_param_t *param)
> +static void t3e3_port_set(struct channel *sc, t3e3_param_t *param)
> {
> if (param->frame_mode != 0xff)
> cpld_set_frame_mode(sc, param->frame_mode);
> @@ -216,7 +216,7 @@ void t3e3_port_set(struct channel *sc, t3e3_param_t
> *param)
> cpld_set_scrambler(sc, param->scrambler);
> }
>
> -void t3e3_port_get_stats(struct channel *sc,
> +static void t3e3_port_get_stats(struct channel *sc,
>t3e3_stats_t *stats)
> {
> u32 result;
> @@ -282,7 +282,7 @@ void t3e3_port_get_stats(struct channel *sc,
> memcpy(stats, &(sc->s), sizeof(t3e3_stats_t));
> }
>
> -void t3e3_port_del_stats(struct channel *sc)
> +static void t3e3_port_del_stats(struct channel *sc)
> {
> memset(&(sc->s), 0, sizeof(t3e3_stats_t));
> }
> --
> 1.7.9.5
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] drivers: dgap: Include appropriate header file in dgap_trace.c
On Sat, Dec 21, 2013 at 03:58:25PM +0530, Rashika Kheria wrote: > Include appropriate header file dgap/dgap_trace.h in dgap_trace.c > because function dgap_tracer_free() have its prototype declaration in > the header file. > > This eliminates the following warning in dgap_trace.c: > drivers/staging/dgap/dgap_trace.c:181:6: warning: no previous prototype for > ‘dgap_tracer_free’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/staging/dgap/dgap_trace.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/dgap/dgap_trace.c > b/drivers/staging/dgap/dgap_trace.c > index 0f9a956..6c8b510 100644 > --- a/drivers/staging/dgap/dgap_trace.c > +++ b/drivers/staging/dgap/dgap_trace.c > @@ -37,6 +37,7 @@ > #include > > #include "dgap_driver.h" > +#include "dgap_trace.h" > > #define TRC_TO_CONSOLE 1 > > -- > 1.7.9.5 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: dgnc: Include appropriate header file in dgnc_trace.c
On Sat, Dec 21, 2013 at 03:52:38PM +0530, Rashika Kheria wrote: > Include appropriate header file dgnc/dgnc_trace.h in dgnc_trace.c > because function dgnc_tracer_free() has its prototype declaration in the > header file. > > This eliminates the following warning in dgnc_trace.c: > drivers/staging/dgnc/dgnc_trace.c:180:6: warning: no previous prototype for > ‘dgnc_tracer_free’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/staging/dgnc/dgnc_trace.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/dgnc/dgnc_trace.c > b/drivers/staging/dgnc/dgnc_trace.c > index a98b7d4..e2164e4 100644 > --- a/drivers/staging/dgnc/dgnc_trace.c > +++ b/drivers/staging/dgnc/dgnc_trace.c > @@ -35,6 +35,7 @@ > #include > > #include "dgnc_driver.h" > +#include "dgnc_trace.h" > > #define TRC_TO_CONSOLE 1 > > -- > 1.7.9.5 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] drivers: bcm: Remove unused function in nvm.c
On Sat, Dec 21, 2013 at 04:03:22PM +0530, Rashika Kheria wrote:
> Remove unused function PropagateCalParamsFromEEPROMToMemory() in nvm.c.
>
> This eliminates the following warning in nvm.c:
> drivers/staging/bcm/nvm.c:1369:5: warning: no previous prototype for
> ‘PropagateCalParamsFromEEPROMToMemory’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
Reviewed-by: Josh Triplett
> drivers/staging/bcm/nvm.c | 61
> -
> 1 file changed, 61 deletions(-)
>
> diff --git a/drivers/staging/bcm/nvm.c b/drivers/staging/bcm/nvm.c
> index 9e5f955..6f40f17 100644
> --- a/drivers/staging/bcm/nvm.c
> +++ b/drivers/staging/bcm/nvm.c
> @@ -1355,67 +1355,6 @@ BeceemFlashBulkWriteStatus_EXIT:
> }
>
> /*
> - * Procedure:PropagateCalParamsFromEEPROMToMemory
> - *
> - * Description: Dumps the calibration section of EEPROM to DDR.
> - *
> - * Arguments:
> - * Adapter- ptr to Adapter object instance
> - * Returns:
> - * OSAL_STATUS_CODE
> - *
> - */
> -
> -int PropagateCalParamsFromEEPROMToMemory(struct bcm_mini_adapter *Adapter)
> -{
> - PCHAR pBuff = kmalloc(BUFFER_4K, GFP_KERNEL);
> - unsigned int uiEepromSize = 0;
> - unsigned int uiIndex = 0;
> - unsigned int uiBytesToCopy = 0;
> - unsigned int uiCalStartAddr = EEPROM_CALPARAM_START;
> - unsigned int uiMemoryLoc = EEPROM_CAL_DATA_INTERNAL_LOC;
> - unsigned int value;
> - int Status = 0;
> -
> - if (!pBuff)
> - return -ENOMEM;
> -
> - if (0 != BeceemEEPROMBulkRead(Adapter, &uiEepromSize,
> EEPROM_SIZE_OFFSET, 4)) {
> - kfree(pBuff);
> - return -1;
> - }
> -
> - uiEepromSize >>= 16;
> - if (uiEepromSize > 1024 * 1024) {
> - kfree(pBuff);
> - return -1;
> - }
> -
> - uiBytesToCopy = MIN(BUFFER_4K, uiEepromSize);
> -
> - while (uiBytesToCopy) {
> - if (0 != BeceemEEPROMBulkRead(Adapter, (PUINT)pBuff,
> uiCalStartAddr, uiBytesToCopy)) {
> - Status = -1;
> - break;
> - }
> - wrm(Adapter, uiMemoryLoc, (PCHAR)(((PULONG)pBuff) + uiIndex),
> uiBytesToCopy);
> - uiMemoryLoc += uiBytesToCopy;
> - uiEepromSize -= uiBytesToCopy;
> - uiCalStartAddr += uiBytesToCopy;
> - uiIndex += uiBytesToCopy / 4;
> - uiBytesToCopy = MIN(BUFFER_4K, uiEepromSize);
> -
> - }
> - value = 0xbeadbead;
> - wrmalt(Adapter, EEPROM_CAL_DATA_INTERNAL_LOC - 4, &value,
> sizeof(value));
> - value = 0xbeadbead;
> - wrmalt(Adapter, EEPROM_CAL_DATA_INTERNAL_LOC - 8, &value,
> sizeof(value));
> - kfree(pBuff);
> -
> - return Status;
> -}
> -
> -/*
> * Procedure:PropagateCalParamsFromFlashToMemory
> *
> * Description: Dumps the calibration section of EEPROM to DDR.
> --
> 1.7.9.5
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] drivers: bcm: Mark functions as static in Qos.c
On Sat, Dec 21, 2013 at 04:01:16PM +0530, Rashika Kheria wrote:
> Mark functions MatchSrcIpAddress(), MatchDestIpAddress() and MatchTos()
> as static in Qos.c because they are not used outside this file.
>
> This eliminates the following warnings in Qos.c:
> drivers/staging/bcm/Qos.c:27:6: warning: no previous prototype for
> ‘MatchSrcIpAddress’ [-Wmissing-prototypes]
> drivers/staging/bcm/Qos.c:61:6: warning: no previous prototype for
> ‘MatchDestIpAddress’ [-Wmissing-prototypes]
> drivers/staging/bcm/Qos.c:94:6: warning: no previous prototype for ‘MatchTos’
> [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
Reviewed-by: Josh Triplett
> drivers/staging/bcm/Qos.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/bcm/Qos.c b/drivers/staging/bcm/Qos.c
> index 1609a2b..0727599 100644
> --- a/drivers/staging/bcm/Qos.c
> +++ b/drivers/staging/bcm/Qos.c
> @@ -24,7 +24,7 @@ static VOID PruneQueue(struct bcm_mini_adapter *Adapter,
> INT iIndex);
> *
> * Returns - TRUE(If address matches) else FAIL .
> */
> -bool MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule, ULONG
> ulSrcIP)
> +static bool MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule,
> ULONG ulSrcIP)
> {
> UCHAR ucLoopIndex = 0;
>
> @@ -58,7 +58,7 @@ bool MatchSrcIpAddress(struct bcm_classifier_rule
> *pstClassifierRule, ULONG ulSr
> *
> * Returns - TRUE(If address matches) else FAIL .
> */
> -bool MatchDestIpAddress(struct bcm_classifier_rule *pstClassifierRule, ULONG
> ulDestIP)
> +static bool MatchDestIpAddress(struct bcm_classifier_rule
> *pstClassifierRule, ULONG ulDestIP)
> {
> UCHAR ucLoopIndex = 0;
> struct bcm_mini_adapter *Adapter = GET_BCM_ADAPTER(gblpnetdev);
> @@ -91,7 +91,7 @@ bool MatchDestIpAddress(struct bcm_classifier_rule
> *pstClassifierRule, ULONG ulD
> *
> * Returns - TRUE(If address matches) else FAIL.
> **/
> -bool MatchTos(struct bcm_classifier_rule *pstClassifierRule, UCHAR
> ucTypeOfService)
> +static bool MatchTos(struct bcm_classifier_rule *pstClassifierRule, UCHAR
> ucTypeOfService)
> {
>
> struct bcm_mini_adapter *Adapter = GET_BCM_ADAPTER(gblpnetdev);
> --
> 1.7.9.5
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/7] Eliminate uses of __DATE__ and __TIME__
Using the __DATE__ and __TIME macros makes the kernel build non-deterministic. The kernel already optionally records this information at build time, so random kernel code shouldn't duplicate that. Eliminate all uses of __DATE__ and __TIME__, and then turn on a new GCC warning if available to make sure no new uses get added. Josh Triplett (7): mtd: denali: Drop print of build date/time net: wireless: brcm80211: Drop debug version with build date/time staging: rtl8188eu: Drop print of build date/time staging: rts5139: Drop print of build time staging: wlags49_h2: Drop debug macro recording build date/time x86: math-emu: Drop already-disabled print of build date Makefile: Build with -Werror=date-time if the compiler supports it Makefile | 3 +++ arch/x86/math-emu/errors.c | 5 - drivers/mtd/nand/denali_pci.c| 1 - drivers/net/wireless/brcm80211/brcmfmac/dhd_common.c | 7 --- drivers/staging/rtl8188eu/os_dep/usb_intf.c | 1 - drivers/staging/rts5139/rts51x_scsi.c| 1 - drivers/staging/wlags49_h2/wl_version.h | 4 7 files changed, 3 insertions(+), 19 deletions(-) -- 1.8.5.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] staging: rts5139: Drop print of build time
The kernel already has this information, and individual drivers
shouldn't duplicate that. This also eliminates the use of __TIME__,
which makes the build non-deterministic. (And, without __DATE__,
__TIME__ provided little useful information to begin with.)
Signed-off-by: Josh Triplett
---
drivers/staging/rts5139/rts51x_scsi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/rts5139/rts51x_scsi.c
b/drivers/staging/rts5139/rts51x_scsi.c
index a474eed..3a99025 100644
--- a/drivers/staging/rts5139/rts51x_scsi.c
+++ b/drivers/staging/rts5139/rts51x_scsi.c
@@ -1985,7 +1985,6 @@ static int show_info(struct seq_file *m, struct Scsi_Host
*host)
SPRINTF(" Vendor: Realtek Corp.\n");
SPRINTF(" Product: RTS51xx USB Card Reader\n");
SPRINTF(" Version: %s\n", DRIVER_VERSION);
- SPRINTF("Build: %s\n", __TIME__);
return 0;
}
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/7] staging: rtl8188eu: Drop print of build date/time
The kernel already has this information, and individual drivers
shouldn't duplicate that. This also eliminates the use of __DATE__ and
__TIME__, which make the build non-deterministic.
Signed-off-by: Josh Triplett
---
drivers/staging/rtl8188eu/os_dep/usb_intf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 7d14779..d44f606 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -865,7 +865,6 @@ static int __init rtw_drv_entry(void)
RT_TRACE(_module_hci_intfs_c_, _drv_err_, ("+rtw_drv_entry\n"));
DBG_88E(DRV_NAME " driver version=%s\n", DRIVERVERSION);
- DBG_88E("build time: %s %s\n", __DATE__, __TIME__);
rtw_suspend_lock_init();
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/7] staging: wlags49_h2: Drop debug macro recording build date/time
The kernel already has this information, and individual drivers shouldn't duplicate that. This also eliminates the use of __DATE__ and __TIME__, which make the build non-deterministic. Signed-off-by: Josh Triplett --- drivers/staging/wlags49_h2/wl_version.h | 4 1 file changed, 4 deletions(-) diff --git a/drivers/staging/wlags49_h2/wl_version.h b/drivers/staging/wlags49_h2/wl_version.h index 037b526..019ebb2 100644 --- a/drivers/staging/wlags49_h2/wl_version.h +++ b/drivers/staging/wlags49_h2/wl_version.h @@ -129,11 +129,7 @@ err: define bus type; #endif /* HERMES25 */ #endif /* BUS_XXX */ -#ifdef DBG -#define MODULE_DATE __DATE__ " " __TIME__ -#else #define MODULE_DATE "07/18/2004 13:30:00" -#endif // DBG //#define STR2(m) #m //#define STR1(m) STR2(m) -- 1.8.5.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: Mark functions as static and remove unused function in bpctl_mod.c
On Fri, Jan 24, 2014 at 04:14:00AM +0530, Rashika Kheria wrote: > Mark functions as static in bpctl_mod.c because they are not used > outside this file. Remove unused function from bpctl_mod.c. > [...] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
imx-hdmi fails to build with v3.13-10094-g9b0cd30
Hi All,
After the DRM merge the imx-hdmi driver fails to build with the error
below. I'm somewhat surprised this wasn't hit in linux-next.
josh
drivers/staging/imx-drm/imx-hdmi.c:55:6: error: nested redefinition of
'enum hdmi_colorimetry'
enum hdmi_colorimetry {
^
drivers/staging/imx-drm/imx-hdmi.c:55:6: error: redeclaration of 'enum
hdmi_colorimetry'
In file included from include/drm/drm_crtc.h:33:0,
from include/drm/drmP.h:711,
from drivers/staging/imx-drm/imx-hdmi.c:24:
include/linux/hdmi.h:48:6: note: originally defined here
enum hdmi_colorimetry {
^
make[3]: *** [drivers/staging/imx-drm/imx-hdmi.o] Error 1
make[3]: *** Waiting for unfinished jobs
make[2]: *** [drivers/staging/imx-drm] Error 2
make[1]: *** [drivers/staging] Error 2
make[1]: *** Waiting for unfinished jobs
make: *** [drivers] Error 2
+ exit 1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: imx-hdmi fails to build with v3.13-10094-g9b0cd30
On Thu, Jan 30, 2014 at 2:31 PM, Fabio Estevam wrote: > On Thu, Jan 30, 2014 at 5:28 PM, Josh Boyer wrote: >> Hi All, >> >> After the DRM merge the imx-hdmi driver fails to build with the error >> below. I'm somewhat surprised this wasn't hit in linux-next. >> > > Sachin has already sent a patch to fix this issue: > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2014-January/045167.html Wonderful. Thanks for the quick response. Out of curiosity, was this hit in linux-next? Or is the config option not enabled in any of the configs built there? josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8188eu: os_dep: usb_intf.c: Fix to remove null pointer checks that could never happen
On Thu, May 22, 2014 at 09:58:02PM +0200, Rickard Strandqvist wrote:
> Removal of null pointer checks that could never happen
>
> Signed-off-by: Rickard Strandqvist
Reviewed-by: Josh Triplett
> drivers/staging/rtl8188eu/os_dep/usb_intf.c | 110
> +--
> 1 file changed, 52 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> index 2e49cd5..3454e1b 100644
> --- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> @@ -396,49 +396,47 @@ int rtw_hw_suspend(struct adapter *padapter)
> goto error_exit;
> }
>
> - if (padapter) { /* system suspend */
> - LeaveAllPowerSaveMode(padapter);
> + /* system suspend */
> + LeaveAllPowerSaveMode(padapter);
>
> - DBG_88E("==> rtw_hw_suspend\n");
> - _enter_pwrlock(&pwrpriv->lock);
> - pwrpriv->bips_processing = true;
> - /* s1. */
> - if (pnetdev) {
> - netif_carrier_off(pnetdev);
> - rtw_netif_stop_queue(pnetdev);
> - }
> + DBG_88E("==> rtw_hw_suspend\n");
> + _enter_pwrlock(&pwrpriv->lock);
> + pwrpriv->bips_processing = true;
> + /* s1. */
> + if (pnetdev) {
> + netif_carrier_off(pnetdev);
> + rtw_netif_stop_queue(pnetdev);
> + }
>
> - /* s2. */
> - rtw_disassoc_cmd(padapter, 500, false);
> + /* s2. */
> + rtw_disassoc_cmd(padapter, 500, false);
>
> - /* s2-2. indicate disconnect to os */
> - {
> - struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> + /* s2-2. indicate disconnect to os */
> + {
> + struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>
> - if (check_fwstate(pmlmepriv, _FW_LINKED)) {
> - _clr_fwstate_(pmlmepriv, _FW_LINKED);
> + if (check_fwstate(pmlmepriv, _FW_LINKED)) {
> + _clr_fwstate_(pmlmepriv, _FW_LINKED);
>
> - rtw_led_control(padapter, LED_CTL_NO_LINK);
> + rtw_led_control(padapter, LED_CTL_NO_LINK);
>
> - rtw_os_indicate_disconnect(padapter);
> + rtw_os_indicate_disconnect(padapter);
>
> - /* donnot enqueue cmd */
> - rtw_lps_ctrl_wk_cmd(padapter,
> LPS_CTRL_DISCONNECT, 0);
> - }
> + /* donnot enqueue cmd */
> + rtw_lps_ctrl_wk_cmd(padapter, LPS_CTRL_DISCONNECT, 0);
> }
> - /* s2-3. */
> - rtw_free_assoc_resources(padapter, 1);
> + }
> + /* s2-3. */
> + rtw_free_assoc_resources(padapter, 1);
>
> - /* s2-4. */
> - rtw_free_network_queue(padapter, true);
> - rtw_ips_dev_unload(padapter);
> - pwrpriv->rf_pwrstate = rf_off;
> - pwrpriv->bips_processing = false;
> + /* s2-4. */
> + rtw_free_network_queue(padapter, true);
> + rtw_ips_dev_unload(padapter);
> + pwrpriv->rf_pwrstate = rf_off;
> + pwrpriv->bips_processing = false;
> +
> + _exit_pwrlock(&pwrpriv->lock);
>
> - _exit_pwrlock(&pwrpriv->lock);
> - } else {
> - goto error_exit;
> - }
> return 0;
>
> error_exit:
> @@ -452,35 +450,32 @@ int rtw_hw_resume(struct adapter *padapter)
> struct net_device *pnetdev = padapter->pnetdev;
>
>
> - if (padapter) { /* system resume */
> - DBG_88E("==> rtw_hw_resume\n");
> - _enter_pwrlock(&pwrpriv->lock);
> - pwrpriv->bips_processing = true;
> - rtw_reset_drv_sw(padapter);
> + /* system resume */
> + DBG_88E("==> rtw_hw_resume\n");
> + _enter_pwrlock(&pwrpriv->lock);
> + pwrpriv->bips_processing = true;
> + rtw_reset_drv_sw(padapter);
>
> - if (pm_netdev_open(pnetdev, false) != 0) {
> - _exit_pwrlock(&pwrpriv->lock);
> - goto error_exit;
> - }
> + if (pm_netdev_open(pnetdev, false) != 0) {
> + _exit_pwrlock(&pwrpriv->lock);
> + goto error_exit;
> + }
>
> - netif_device_attach(pnetdev);
> - netif_carrier_on(pnetdev
Re: [PATCH] Staging/comedi: Fixes static analysis warning raised by sparse
On Sat, Jun 28, 2014 at 11:07:48AM -0700, Christopher Li wrote:
> On Wed, Jun 11, 2014 at 2:45 PM, wrote:
> > On Thu, Jun 12, 2014 at 12:24:25AM +0300, Dan Carpenter wrote:
> >> Let's forward this to the Sparse mailing list.
> >>
> >> We're seeing a Sparse false positive testing
>
> No, this is a valid complain. See my point follows.
>
> >> drivers/staging/comedi/drivers/ni_pcimio.c.
> >>
> >> CHECK drivers/staging/comedi/drivers/ni_pcimio.c
> >> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> >> (4294967295) for type int
> >> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> >> (4294967295) for type int
> >> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> >> (4294967295) for type int
> >> drivers/staging/comedi/drivers/ni_stc.h:720:26: warning: shift too big
> >> (4294967295) for type int
> >>
> >> I have created some test code to demonstrate the problem (attached).
> >>
> >> The check_shift_count() warning is only supposed to be printed for
> >> number literals but because of the way inline functions are expanded it
> >> still complains even though channel is a variable.
> >
> > Thanks for the test case; this definitely makes no sense. I don't think
> > Sparse will suddenly develop enough range analysis or reachability
> > analysis to handle this case; I think the right answer is to avoid
> > giving such warnings for shifts with a non-constant RHS.
>
> Sparse can handle inline function expand and some constant
> propagate. In this case, sparse seems doing the right thing.
> Sparse actually sees the channel value being 4294967295 (-1).
>
> >> static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
>
> This is the bug. See this channel is *unsigned*. When -1 pass into
> channel, it become a really large number 4294967295.
> The code does request C compiler to perform left shift 4294967295 bits.
> Which did not make sense.
>
> >> {
> >> if (channel < 4)
> >> return 1 << channel;
> >> return 0;
> >> }
> >>
> >> static inline void filter(int channel)
> >> {
> >> if (channel < 0)
> >> return;
> >> ni_stc_dma_channel_select_bitfield(channel);
>
> See this channel is *signed*, with -1 convert to 4294967295.
> This is a bug in the kernel source not sparse.
Except that "filter" has an "int channel" (signed), so it can
successfully test "channel < 0" and return early; it'll never call
ni_stc_dma_channel_select_bitfield(channel) with a negative number.
I do agree that this code should sort out the signedness of its types,
but in this case I don't think the bad shift can actually happen, making
this a false positive.
- Josh Triplett
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 4/4] fpga manager: add driver for socfpga fpga manager
On Tue, Sep 22, 2015 at 10:21:11AM -0500, at...@opensource.altera.com wrote:
> From: Alan Tull
>
> Add driver to fpga manager framework to allow configuration
> of FPGA in Altera SoCFPGA parts.
>
> Signed-off-by: Alan Tull
> Acked-by: Michal Simek
> Acked-by: Moritz Fischer
[..]
> +++ b/drivers/fpga/Kconfig
> @@ -11,4 +11,14 @@ config FPGA
> kernel. The FPGA framework adds a FPGA manager class and FPGA
> manager drivers.
>
> +if FPGA
FPGA is unconditionally set here, otherwise drivers/fpga/Kconfig
wouldn't even be considered.
> +
> +config FPGA_MGR_SOCFPGA
> + tristate "Altera SOCFPGA FPGA Manager"
> + depends on ARCH_SOCFPGA
> + help
> + FPGA manager driver support for Altera SOCFPGA.
> +
> +endif # FPGA
> +
> endmenu
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 3313c52..ba6c5c5 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -6,3 +6,4 @@
> obj-$(CONFIG_FPGA) += fpga-mgr.o
>
> # FPGA Manager Drivers
> +obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
> new file mode 100644
> index 000..706b80d
> --- /dev/null
> +++ b/drivers/fpga/socfpga.c
[..]
> +/*
> + * Step 9: write data to the FPGA data register
> + */
> +static int socfpga_fpga_ops_configure_write(struct fpga_manager *mgr,
> + const char *buf, size_t count)
> +{
> + struct socfpga_fpga_priv *priv = mgr->priv;
> + u32 *buffer_32 = (u32 *)buf;
Seems sketchy from an endianess perspective, but it may be okay if
SOCFPGA doesn't support BE (which my follow up question would be: why
not?). Same thing applies with seemingly cavalier usages of the
__raw_readl/writel variants.
Josh
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 3/4] add FPGA manager core
R_RO(name);
> +static DEVICE_ATTR_RO(state);
> +
> +static struct attribute *fpga_mgr_attrs[] = {
> + &dev_attr_name.attr,
> + &dev_attr_state.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(fpga_mgr);
> +
> +static int fpga_mgr_of_node_match(struct device *dev, const void *data)
> +{
> + return dev->of_node == data;
> +}
> +
> +/**
> + * of_fpga_mgr_get - get an exclusive reference to a fpga mgr
> + * @node:device node
> + *
> + * Given a device node, get an exclusive reference to a fpga mgr.
> + *
> + * Return: fpga manager struct or IS_ERR() condition containing error code.
> + */
> +struct fpga_manager *of_fpga_mgr_get(struct device_node *node)
> +{
> + struct fpga_manager *mgr;
> + struct device *dev;
> +
> + if (!node)
> + return ERR_PTR(-EINVAL);
> +
> + dev = class_find_device(fpga_mgr_class, NULL, node,
> + fpga_mgr_of_node_match);
> + if (!dev)
> + return ERR_PTR(-ENODEV);
> +
> + mgr = to_fpga_manager(dev);
> + put_device(dev);
Who's ensuring the FPGA manager device's lifetime between _get and _put?
> + if (!mgr)
> + return ERR_PTR(-ENODEV);
> +
> + /* Get exclusive use of fpga manager */
> + if (!mutex_trylock(&mgr->ref_mutex))
> + return ERR_PTR(-EBUSY);
> +
> + if (!try_module_get(THIS_MODULE)) {
> + mutex_unlock(&mgr->ref_mutex);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + return mgr;
> +}
[..]
> +int fpga_mgr_register(struct device *dev, const char *name,
> + const struct fpga_manager_ops *mops,
> + void *priv)
> +{
> + struct fpga_manager *mgr;
> + const char *dt_label;
> + int id, ret;
> +
> + if (!mops || !mops->write_init || !mops->write ||
> + !mops->write_complete || !mops->state) {
> + dev_err(dev, "Attempt to register without fpga_manager_ops\n");
> + return -EINVAL;
> + }
> +
> + if (!name || !strlen(name)) {
> + dev_err(dev, "Attempt to register with no name!\n");
> + return -EINVAL;
> + }
> +
> + mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
> + if (!mgr)
> + return -ENOMEM;
> +
> + id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
> + if (id < 0) {
> + ret = id;
> + goto error_kfree;
> + }
> +
> + mutex_init(&mgr->ref_mutex);
> +
> + mgr->name = name;
> + mgr->mops = mops;
> + mgr->priv = priv;
> +
> + /*
> + * Initialize framework state by requesting low level driver read state
> + * from device. FPGA may be in reset mode or may have been programmed
> + * by bootloader or EEPROM.
> + */
> + mgr->state = mgr->mops->state(mgr);
> +
> + device_initialize(&mgr->dev);
> + mgr->dev.class = fpga_mgr_class;
> + mgr->dev.parent = dev;
> + mgr->dev.of_node = dev->of_node;
> + mgr->dev.id = id;
> + dev_set_drvdata(dev, mgr);
> +
> + dt_label = of_get_property(mgr->dev.of_node, "label", NULL);
> + if (dt_label)
> + ret = dev_set_name(&mgr->dev, "%s", dt_label);
> + else
> + ret = dev_set_name(&mgr->dev, "fpga%d", id);
I'm wondering if an alias {} node is better for this.
[..]
> +++ b/include/linux/fpga/fpga-mgr.h
[..]
> +/*
> + * FPGA Manager flags
> + * FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
> + */
> +#define FPGA_MGR_PARTIAL_RECONFIGBIT(0)
> +
> +/**
> + * struct fpga_manager_ops - ops for low level fpga manager drivers
> + * @state: returns an enum value of the FPGA's state
> + * @write_init: prepare the FPGA to receive confuration data
^configuration
> + * @write: write count bytes of configuration data to the FPGA
> + * @write_complete: set FPGA to operating state after writing is done
> + * @fpga_remove: optional: Set FPGA into a specific state during driver
> remove
Any specific state? State in the FPGA-manager-state-machine case? Or a
basic 'reset' state?
Josh
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 3/4] add FPGA manager core
On Wed, Sep 23, 2015 at 12:10:13PM -0500, atull wrote:
> On Tue, 22 Sep 2015, Josh Cartwright wrote:
[..]
> > > +struct fpga_manager *of_fpga_mgr_get(struct device_node *node)
> > > +{
> > > + struct fpga_manager *mgr;
> > > + struct device *dev;
> > > +
> > > + if (!node)
> > > + return ERR_PTR(-EINVAL);
> > > +
> > > + dev = class_find_device(fpga_mgr_class, NULL, node,
> > > + fpga_mgr_of_node_match);
> > > + if (!dev)
> > > + return ERR_PTR(-ENODEV);
> > > +
> > > + mgr = to_fpga_manager(dev);
> > > + put_device(dev);
> >
> > Who's ensuring the FPGA manager device's lifetime between _get and _put?
>
> Well... get_device and put_device are not sufficient?
Sorry if I was too opaque.
You've just put_device()'d the only reference to the device you had
here, so nothing is preventing from the device from being ripped away
while it's being used.
You should remove the put_device() call here, and add a corresponding
put_device() in fpga_mgr_put().
The module refcounting is also a bit strange, as you are
acquiring/releasing a reference to THIS_MODULE, but you also should care
about the lower-level driver's module refcount.
[..]
> > > + dt_label = of_get_property(mgr->dev.of_node, "label", NULL);
> > > + if (dt_label)
> > > + ret = dev_set_name(&mgr->dev, "%s", dt_label);
> > > + else
> > > + ret = dev_set_name(&mgr->dev, "fpga%d", id);
> >
> > I'm wondering if an alias {} node is better for this.
>
> I could look into that. Is there an example of that you particularly
> like for this?
Look at i2c's usage of the of_alias_*() functions.
Josh
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/9] use c99 initializers in structures
On Sat, Aug 23, 2014 at 01:20:22PM +0200, Julia Lawall wrote: > These patches add labels in the initializations of structure fields (c99 > initializers). The complete semantic patch thta makes this change is shown > below. This rule ignores cases where the initialization is just 0 or NULL, > where some of the fields already use labels, and where there are nested > structures. I responded to patches 6 and 8 with comments; for the rest (1-5, 7, 9): Reviewed-by: Josh Triplett ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] linux-firmware: update wfx
On Mon, Feb 21, 2022 at 11:38 AM Jerome Pouiller
wrote:
>
> From: Jérôme Pouiller
>
> The two first patches reflect changes made in the kernel.
>
> Jérôme Pouiller (3):
> wfx: rename silabs/ into wfx/
> wfx: add antenna configuration files
> wfx: update to firmware 3.14
All 3 applied and pushed out.
josh
>
> WHENCE| 18 ++
> silabs/wfm_wf200_C0.sec | Bin 305232 -> 0 bytes
> {silabs => wfx}/LICENCE.wf200 | 0
> wfx/brd4001a.pds | Bin 0 -> 625 bytes
> wfx/brd8022a.pds | Bin 0 -> 584 bytes
> wfx/brd8023a.pds | Bin 0 -> 625 bytes
> wfx/wfm_wf200_C0.sec | Bin 0 -> 309328 bytes
> 7 files changed, 14 insertions(+), 4 deletions(-)
> delete mode 100644 silabs/wfm_wf200_C0.sec
> rename {silabs => wfx}/LICENCE.wf200 (100%)
> create mode 100644 wfx/brd4001a.pds
> create mode 100644 wfx/brd8022a.pds
> create mode 100644 wfx/brd8023a.pds
> create mode 100644 wfx/wfm_wf200_C0.sec
>
> --
> 2.34.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] wfx: add antenna configuration files
On Thu, Jul 7, 2022 at 1:04 PM Ben Brown wrote: > > On 21/02/2022 16:37, Jerome Pouiller wrote: > > From: Jérôme Pouiller > > > diff --git a/WHENCE b/WHENCE > > index 0a6cb15..96f67f7 100644 > > --- a/WHENCE > > +++ b/WHENCE > > @@ -5845,8 +5845,18 @@ Driver: wfx - Silicon Labs Wi-Fi Transceiver > > File: wfx/wfm_wf200_C0.sec > > Version: 3.12.1 > > > > +File: wfx/brd4001a.pds not listed in WHENCE > > +File: wfx/brd8022a.pds not listed in WHENCE > > +File: wfx/brd8023a.pds not listed in WHENCE > > This format does not appear to be correct. While this will seemingly > pass the `check_whence.py` check, it will be completely ignored by > `copy-firmware.sh`, as that takes the full line after 'File: ' (e.g. > 'wfx/brd4001a.pds not listed in WHENCE', which of course does not exist). Oh, indeed. > I'm assuming the trailing ' not listed in WHENCE' needs to be removed > from each of these lines. Otherwise these are likely not being picked up > by distros (they are missing from Arch, for example). This may have been > the intention, but that seems odd (and unclear if so). I doubt that was the intention. I'll correct WHENCE in a separate commit. Thank you for reporting the issue. josh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
