From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932355AbcCIMJV (ORCPT ); Wed, 9 Mar 2016 07:09:21 -0500 Received: from smtp121.iad3a.emailsrvr.com ([173.203.187.121]:50652 "EHLO smtp121.iad3a.emailsrvr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932298AbcCIMJI (ORCPT ); Wed, 9 Mar 2016 07:09:08 -0500 X-Auth-ID: abbotti@mev.co.uk X-Sender-Id: abbotti@mev.co.uk Subject: Re: [PATCH 1/1] Staging: comedi: fix CHECK issue in ni_tio.c To: ravishankarkm , greg@kroah.com References: <1457523302-21261-1-git-send-email-ravishankarkm32@gmail.com> Cc: hsweeten@visionengravers.com, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org From: Ian Abbott Message-ID: <56E01261.6070600@mev.co.uk> Date: Wed, 9 Mar 2016 12:09:05 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: <1457523302-21261-1-git-send-email-ravishankarkm32@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/03/16 11:35, ravishankarkm wrote: > This is a patch to the ni_tio.c file that fixes up a CHECK issues > found by the checkpatch.pl tool. > > i.e.Avoid crashing the kernel - try using WARN_ON & recovery code > rather than BUG() or BUG_ON(). > > Signed-off-by: Ravishankar Karkala Mallikarjunayya > --- > drivers/staging/comedi/drivers/ni_tio.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > I guess this should be PATCH 1/2, not PATCH 1/1? > diff --git a/drivers/staging/comedi/drivers/ni_tio.c b/drivers/staging/comedi/drivers/ni_tio.c > index b74e44e..1348a75 100644 > --- a/drivers/staging/comedi/drivers/ni_tio.c > +++ b/drivers/staging/comedi/drivers/ni_tio.c > @@ -189,7 +189,7 @@ static void ni_tio_reset_count_and_disarm(struct ni_gpct *counter) > static uint64_t ni_tio_clock_period_ps(const struct ni_gpct *counter, > unsigned generic_clock_source) > { > - uint64_t clock_period_ps; > + u64 clock_period_ps; This change from uint64_t to u64 comes from your PATCH 2/2, but it has got mixed into this patch too. That makes it impossible to apply both of your patches. > > switch (generic_clock_source & NI_GPCT_CLOCK_SRC_SELECT_MASK) { > case NI_GPCT_TIMEBASE_1_CLOCK_SRC_BITS: > @@ -222,7 +222,7 @@ static uint64_t ni_tio_clock_period_ps(const struct ni_gpct *counter, > clock_period_ps *= 8; > break; > default: > - BUG(); > + WARN_ON(1); These BUG() to WARN_ON(1) changes are good. > break; > } > return clock_period_ps; > @@ -304,7 +304,7 @@ static unsigned ni_m_series_clock_src_select(const struct ni_gpct *counter) > } > if (i <= NI_M_MAX_PFI_CHAN) > break; > - BUG(); > + WARN_ON(1); > break; > } > clock_source |= ni_tio_clock_src_modifiers(counter); > @@ -361,7 +361,7 @@ static unsigned ni_660x_clock_src_select(const struct ni_gpct *counter) > } > if (i <= NI_660X_MAX_SRC_PIN) > break; > - BUG(); > + WARN_ON(1); > break; > } > clock_source |= ni_tio_clock_src_modifiers(counter); > @@ -385,9 +385,9 @@ static void ni_tio_set_sync_mode(struct ni_gpct *counter, int force_alt_sync) > struct ni_gpct_device *counter_dev = counter->counter_dev; > unsigned cidx = counter->counter_index; > const unsigned counting_mode_reg = NITIO_CNT_MODE_REG(cidx); > - static const uint64_t min_normal_sync_period_ps = 25000; > + static const u64 min_normal_sync_period_ps = 25000; > unsigned mode; > - uint64_t clock_period_ps; > + u64 clock_period_ps; > > if (ni_tio_counting_mode_registers_present(counter_dev) == 0) > return; > @@ -584,7 +584,7 @@ static unsigned ni_660x_clk_src(unsigned int clock_source) > if (i <= NI_660X_MAX_SRC_PIN) > break; > ni_660x_clock = 0; > - BUG(); > + WARN_ON(1); > break; > } > return GI_SRC_SEL(ni_660x_clock); > @@ -643,7 +643,7 @@ static unsigned ni_m_clk_src(unsigned int clock_source) > break; > pr_err("invalid clock source 0x%lx\n", > (unsigned long)clock_source); > - BUG(); > + WARN_ON(1); > ni_m_series_clock = 0; > break; > } > @@ -730,7 +730,7 @@ static void ni_tio_get_clock_src(struct ni_gpct *counter, > unsigned int *clock_source, > unsigned int *period_ns) > { > - uint64_t temp64; > + u64 temp64; > > *clock_source = ni_tio_generic_clock_src_select(counter); > temp64 = ni_tio_clock_period_ps(counter, *clock_source); > @@ -946,7 +946,7 @@ int ni_tio_set_gate_src(struct ni_gpct *counter, unsigned gate_index, > case ni_gpct_variant_660x: > return ni_660x_set_gate2(counter, gate_source); > default: > - BUG(); > + WARN_ON(1); > break; > } > break; > @@ -1015,7 +1015,7 @@ static unsigned ni_660x_gate_to_generic_gate(unsigned gate) > if (gate == NI_660X_PIN_GATE_SEL(i)) > return NI_GPCT_GATE_PIN_GATE_SELECT(i); > } > - BUG(); > + WARN_ON(1); > break; > } > return 0; > @@ -1051,7 +1051,7 @@ static unsigned ni_m_gate_to_generic_gate(unsigned gate) > if (gate == NI_M_PFI_GATE_SEL(i)) > return NI_GPCT_PFI_GATE_SELECT(i); > } > - BUG(); > + WARN_ON(1); > break; > } > return 0; > @@ -1083,7 +1083,7 @@ static unsigned ni_660x_gate2_to_generic_gate(unsigned gate) > if (gate == NI_660X_UD_PIN_GATE2_SEL(i)) > return NI_GPCT_UP_DOWN_PIN_GATE_SELECT(i); > } > - BUG(); > + WARN_ON(1); > break; > } > return 0; > @@ -1119,7 +1119,7 @@ static int ni_tio_get_gate_src(struct ni_gpct *counter, unsigned gate_index, > } > > gate = GI_BITS_TO_GATE(ni_tio_get_soft_copy(counter, > - NITIO_INPUT_SEL_REG(cidx))); > + NITIO_INPUT_SEL_REG(cidx))); There is no need for the patch to adjust the whitespace on that line. Ideally, the 'NITIO_INPUT_SEL_REG(cidx)' should line up with 'counter' on the previous line, but that would take the line over 80 characters. The current amount of whitespace is a compromise, but there is no need to change it. -- -=( Ian Abbott @ MEV Ltd. E-mail: )=- -=( Web: http://www.mev.co.uk/ )=-