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

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

[Joe Perches]

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

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

[James Kosin]

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

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

[James Kosin]

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

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

[James Kosin]

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

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

[Nick Bowler]

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/)