Re: [PATCH 23/24] drivers/block/floppy.c: Add function is_ready

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* Re: [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state
       [not found] ` <e5jMM-25X-51@gated-at.bofh.it>
@ 2010-01-23 17:32   ` James Kosin
       [not found]     ` <1264279618.30778.34.camel@Joe-Laptop.home>
  0 siblings, 1 reply; 5+ messages in thread
From: James Kosin @ 2010-01-23 17:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: joe

On 1/22/2010 12:00 AM, Joe Perches wrote:
> Used a couple of times, might simplify the code a bit.
>
> Signed-off-by: Joe Perches<joe@perches.com>
> ---
>   drivers/block/floppy.c |   19 +++++++++++++++----
>   1 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 2f6ed78..fd56b26 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -782,6 +782,12 @@ static inline int is_selected(int dor, int unit)
>   	return ((dor&  (0x10<<  unit))&&  (dor&  3) == unit);
>   }
>
> +static bool is_ready_state(int status)
> +{
> +	int state = status&  (STATUS_READY | STATUS_DIR | STATUS_DMA);
> +	return state == STATUS_READY;
> +}
> +

This should probably be simplified to:

static bool is_ready_state(int status)
{
	return ((state & STATUS_READY) == STATUS_READY);
}

James

^ permalink raw reply	[flat|nested] 5+ messages in thread

[parent not found: <1264279618.30778.34.camel@Joe-Laptop.home>]

* Re: [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state
       [not found]     ` <1264279618.30778.34.camel@Joe-Laptop.home>
@ 2010-01-26  1:26       ` James Kosin
  2010-01-26 15:06         ` Nick Bowler
  2010-01-26  1:34       ` James Kosin
  1 sibling, 1 reply; 5+ messages in thread
From: James Kosin @ 2010-01-26  1:26 UTC (permalink / raw)
  To: Joe Perches, linux-kernel

On 1/23/2010 3:46 PM, Joe Perches wrote:
> On Sat, 2010-01-23 at 12:32 -0500, James Kosin wrote:
>    
>> On 1/22/2010 12:00 AM, Joe Perches wrote:
>>      
>>> Used a couple of times, might simplify the code a bit.
>>>
>>> Signed-off-by: Joe Perches<joe@perches.com>
>>> ---
>>>    drivers/block/floppy.c |   19 +++++++++++++++----
>>>    1 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
>>> index 2f6ed78..fd56b26 100644
>>> --- a/drivers/block/floppy.c
>>> +++ b/drivers/block/floppy.c
>>> @@ -782,6 +782,12 @@ static inline int is_selected(int dor, int unit)
>>>    	return ((dor&   (0x10<<   unit))&&   (dor&   3) == unit);
>>>    }
>>>
>>> +static bool is_ready_state(int status)
>>> +{
>>> +	int state = status&   (STATUS_READY | STATUS_DIR | STATUS_DMA);
>>> +	return state == STATUS_READY;
>>> +}
>>> +
>>>        
>> This should probably be simplified to:
>>
>> static bool is_ready_state(int status)
>> {
>> 	return ((state&  STATUS_READY) == STATUS_READY);
>> }
>>      
> Certainly not.
> That wouldn't be the same code.
>
> include/linux/fdreg.h:#define STATUS_DMA	0x20		/* 0- DMA mode */
> include/linux/fdreg.h:#define STATUS_DIR	0x40		/* 0- cpu->fdc */
> include/linux/fdreg.h:#define STATUS_READY	0x80		/* Data reg ready */
>
>
>    
Read the code....

It simplifies what is already there.  The two other status flags make no 
difference in the test for equality with STATUS_READY.

James

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state
  2010-01-26  1:26       ` James Kosin
@ 2010-01-26 15:06         ` Nick Bowler
  0 siblings, 0 replies; 5+ messages in thread
From: Nick Bowler @ 2010-01-26 15:06 UTC (permalink / raw)
  To: James Kosin; +Cc: Joe Perches, linux-kernel

On 20:26 Mon 25 Jan     , James Kosin wrote:
> On 1/23/2010 3:46 PM, Joe Perches wrote:
> > On Sat, 2010-01-23 at 12:32 -0500, James Kosin wrote:
> >    
> >> On 1/22/2010 12:00 AM, Joe Perches wrote:
> >>      
> >>> +static bool is_ready_state(int status)
> >>> +{
> >>> +	int state = status&   (STATUS_READY | STATUS_DIR | STATUS_DMA);
> >>> +	return state == STATUS_READY;
> >>> +}
> >>> +
> >>>        
> >> This should probably be simplified to:
> >>
> >> static bool is_ready_state(int status)
> >> {
> >> 	return ((state&  STATUS_READY) == STATUS_READY);
> >> }
> >>      
> > Certainly not.
> > That wouldn't be the same code.
> >
> > include/linux/fdreg.h:#define STATUS_DMA	0x20		/* 0- DMA mode */
> > include/linux/fdreg.h:#define STATUS_DIR	0x40		/* 0- cpu->fdc */
> > include/linux/fdreg.h:#define STATUS_READY	0x80		/* Data reg ready */
> >    
> Read the code....
> 
> It simplifies what is already there.  The two other status flags make no 
> difference in the test for equality with STATUS_READY.

Sure they do: the old code tests that the STATUS_READY is set and
both STATUS_DMA and STATUS_DIR bits are clear.  So, for example, if
STATUS_READY and STATUS_DMA are both set, then the old test will return
false.

Your "simplification" has removed the check for those clear bits, and
will return true even if all three bits are set.

-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state
       [not found]     ` <1264279618.30778.34.camel@Joe-Laptop.home>
  2010-01-26  1:26       ` James Kosin
@ 2010-01-26  1:34       ` James Kosin
  1 sibling, 0 replies; 5+ messages in thread
From: James Kosin @ 2010-01-26  1:34 UTC (permalink / raw)
  To: Joe Perches, linux-kernel

On 1/23/2010 3:46 PM, Joe Perches wrote:
> On Sat, 2010-01-23 at 12:32 -0500, James Kosin wrote:
>    
>> On 1/22/2010 12:00 AM, Joe Perches wrote:
>>      
>>> Used a couple of times, might simplify the code a bit.
>>>
>>> Signed-off-by: Joe Perches<joe@perches.com>
>>> ---
>>>    drivers/block/floppy.c |   19 +++++++++++++++----
>>>    1 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
>>> index 2f6ed78..fd56b26 100644
>>> --- a/drivers/block/floppy.c
>>> +++ b/drivers/block/floppy.c
>>> @@ -782,6 +782,12 @@ static inline int is_selected(int dor, int unit)
>>>    	return ((dor&   (0x10<<   unit))&&   (dor&   3) == unit);
>>>    }
>>>
>>> +static bool is_ready_state(int status)
>>> +{
>>> +	int state = status&   (STATUS_READY | STATUS_DIR | STATUS_DMA);
>>> +	return state == STATUS_READY;
>>> +}
>>> +
>>>        
>> This should probably be simplified to:
>>
>> static bool is_ready_state(int status)
>> {
>> 	return ((state&  STATUS_READY) == STATUS_READY);
>> }
>>      
> Certainly not.
> That wouldn't be the same code.
>
> include/linux/fdreg.h:#define STATUS_DMA	0x20		/* 0- DMA mode */
> include/linux/fdreg.h:#define STATUS_DIR	0x40		/* 0- cpu->fdc */
> include/linux/fdreg.h:#define STATUS_READY	0x80		/* Data reg ready */
>
>
>    
   Sorry,

That probably should have been:
         return ((status & STATUS_READY) == STATUS_READY);

James

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 00/24] drivers/block/floppy.c cleanups
@ 2010-01-22  4:52 Joe Perches
  2010-01-22  4:52 ` [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2010-01-22  4:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Marcin Slusarz, Stephen Hemminger, Bartlomiej Zolnierkiewicz,
	Andrew Morton

Since Stephen Hemminger is a bit too busy to do much with this
I've broken up the first large patch into multiple pieces and
brought the rest up to current.

I dropped the c99 conversions because Bartlomiej Zolnierkiewicz
objected.

This applies against Linus' and -next.

Maybe somebody will decide it's useful.

Original submissions:
http://lkml.org/lkml/2009/11/30/511
http://lkml.org/lkml/2009/12/2/14

Joe Perches (24):
  drivers/block/floppy.c: Convert some #include <asm/ to #include <linux/
  drivers/block/floppy.c: #define space and column neatening
  drivers/block/floppy.c: Use pr_<level>
  drivers/block/floppy.c: Remove unnecessary braces
  drivers/block/floppy.c: Remove used once CHECK_READY macro
  drivers/block/floppy.c: Hoist assigns from if()s, neatening
  drivers/block/floppy.c: Remove LAST_OUT macro
  drivers/block/floppy.c: Comment neatening and remove naked ;
  drivers/block/floppy.c: Remove CLEARSTRUCT macro, use memset
  drivers/block/floppy.c: Indent a comment
  drivers/block/floppy.c: Remove IN/OUT macros, indent switch/case
  drivers/block/floppy.c: Remove a few spaces from function casts
  drivers/block/floppy.c: Remove macro LOCK_FDC
  drivers/block/floppy.c: Add debug_dcl(...) macro
  drivers/block/floppy.c: Remove [U]CLEARF, [U]SETF, and [U]TESTF macros
  drivers/block/floppy.c: Remove most uses of CALL and ECALL macros
  drivers/block/floppy.c: remove [_]COPYIN [_]COPYOUT and ECALL macros
  drivers/block/floppy.c: remove macros CALL, WAIT and IWAIT
  drivers/block/floppy.c: convert int 1/0 to bool true/false
  drivers/block/floppy.c: Move leading && and || to preceding line
  drivers/block/floppy.c: remove #define DEVICE_NAME "floppy"
  drivers/block/floppy.c: Convert int initialising to bool initialized
  drivers/block/floppy.c: Add function is_ready_state
  drivers/block/floppy.c: Remove unnecessary return and braces

 drivers/block/floppy.c | 1219 ++++++++++++++++++++++++------------------------
 1 files changed, 618 insertions(+), 601 deletions(-)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state
  2010-01-22  4:52 [PATCH 00/24] drivers/block/floppy.c cleanups Joe Perches
@ 2010-01-22  4:52 ` Joe Perches
  0 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2010-01-22  4:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Marcin Slusarz, Stephen Hemminger, Bartlomiej Zolnierkiewicz,
	Andrew Morton

Used a couple of times, might simplify the code a bit.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/block/floppy.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 2f6ed78..fd56b26 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -782,6 +782,12 @@ static inline int is_selected(int dor, int unit)
 	return ((dor & (0x10 << unit)) && (dor & 3) == unit);
 }
 
+static bool is_ready_state(int status)
+{
+	int state = status & (STATUS_READY | STATUS_DIR | STATUS_DMA);
+	return state == STATUS_READY;
+}
+
 static int set_dor(int fdc, char mask, char data)
 {
 	unsigned char unit;
@@ -823,8 +829,10 @@ static void twaddle(void)
 	DRS->select_date = jiffies;
 }
 
-/* reset all driver information about the current fdc. This is needed after
- * a reset, and after a raw command. */
+/*
+ * Reset all driver information about the current fdc.
+ * This is needed after a reset, and after a raw command.
+ */
 static void reset_fdc_info(int mode)
 {
 	int drive;
@@ -1162,7 +1170,8 @@ static int output_byte(char byte)
 
 	if (status < 0)
 		return -1;
-	if ((status & (STATUS_READY | STATUS_DIR | STATUS_DMA)) == STATUS_READY) {
+
+	if (is_ready_state(status)) {
 		fd_outb(byte, FD_DATA);
 #ifdef FLOPPY_SANITY_CHECK
 		output_log[output_log_pos].data = byte;
@@ -1221,8 +1230,10 @@ static int need_more_output(void)
 
 	if (status < 0)
 		return -1;
-	if ((status & (STATUS_READY | STATUS_DIR | STATUS_DMA)) == STATUS_READY)
+
+	if (is_ready_state(status))
 		return MORE_OUTPUT;
+
 	return result();
 }
 
-- 
1.6.6.rc0.57.gad7a


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-01-26 15:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <e5jMK-25X-3@gated-at.bofh.it>
     [not found] ` <e5jMM-25X-51@gated-at.bofh.it>
2010-01-23 17:32   ` [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state James Kosin
     [not found]     ` <1264279618.30778.34.camel@Joe-Laptop.home>
2010-01-26  1:26       ` James Kosin
2010-01-26 15:06         ` Nick Bowler
2010-01-26  1:34       ` James Kosin
2010-01-22  4:52 [PATCH 00/24] drivers/block/floppy.c cleanups Joe Perches
2010-01-22  4:52 ` [PATCH 23/24] drivers/block/floppy.c: Add function is_ready_state Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox