public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Geoff Levand <geoffrey.levand@am•sony.com>
To: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom•com>
Cc: linuxppc-dev@ozlabs•org, paulus@samba•org
Subject: Re: [patch 6/6] PS3: Add os-area database routines
Date: Mon, 08 Oct 2007 18:08:27 -0700	[thread overview]
Message-ID: <470AD48B.5060102@am.sony.com> (raw)
In-Reply-To: <Pine.LNX.4.62.0710081019030.21197@pademelon.sonytel.be>

Geert Uytterhoeven wrote:
> On Sat, 6 Oct 2007 geoffrey.levand@am•sony.com wrote:
>> --- a/arch/powerpc/platforms/ps3/os-area.c
>> +++ b/arch/powerpc/platforms/ps3/os-area.c
>> @@ -112,10 +114,91 @@ struct os_area_params {
>>  	u8 _reserved_5[8];
>>  };
>>  
>> +/**
>> + * struct os_area_db - Shared flash memory database.
>> + * @magic_num: Always '-db-' = 0x2d64622d.
>                                   ^^^^^^^^^^
> #define?


Well, this is a comment, and when debugging it is handy to
know the value.


>> @@ -242,6 +325,303 @@ static int __init verify_header(const st
>>  	return 0;
>>  }
>>  
>> +static int db_verify(const struct os_area_db *db)
>> +{
>> +	if (db->magic_num != 0x2d64622dU) {
>                              ^^^^^^^^^^^
> #define?


Sure, it is OK here.


>> +static void os_area_db_init(struct os_area_db *db)
>> +{
>> +	/*
>> +	 * item      | start | size
>> +	 * ----------+-------+-------
>> +	 * header    | 0     | 24
>> +	 * index_64  | 24    | 64
>> +	 * values_64 | 88    | 57*8 = 456
>> +	 * index_32  | 544   | 64
>> +	 * values_32 | 609   | 57*4 = 228
>> +	 * index_16  | 836   | 64
>> +	 * values_16 | 900   | 57*2 = 114
>> +	 * end       | 1014  | -
>> +	 */
> 
> Lots of #defines and calculations?


OK.


>> +
>> +	memset(db, 0, sizeof(struct os_area_db));
>> +
>> +	db->magic_num = 0x2d64622dU;
>                         ^^^^^^^^^^^
> #define?
> 
>> +	db->version = 1;
>> +	db->index_64 = 24;
>                        ^^
>> +	db->count_64 = 57;
>                        ^^
>> +	db->index_32 = 544;
>                        ^^^
>> +	db->count_32 = 57;
>                        ^^
>> +	db->index_16 = 836;
>                        ^^^
>> +	db->count_16 = 57;
>                        ^^
> #defines?
> 
>> +static void update_flash_db(void)
>> +{
>> +	int result;
>> +	int file;
>> +	off_t offset;
>> +	ssize_t count;
>> +	static const unsigned int buf_len = 8 * OS_AREA_SEGMENT_SIZE;
>> +	const struct os_area_header *header;
>> +	struct os_area_db* db;
>> +
>> +	/* Read in header and db from flash. */
>> +
>> +	file = sys_open("/dev/ps3flash", O_RDWR, 0);
> 
> Ah, file operations from kernel space...


Yes.  I was thinking we could make an interface to the flash driver.

 
>> @@ -264,6 +644,9 @@ static void os_area_queue_work_handler(s
>>  		pr_debug("%s:%d of_find_node_by_path failed\n",
>>  			__func__, __LINE__);
>>  
>> +#if defined(CONFIG_PS3_FLASH) || defined(CONFIG_PS3_FLASH_MODULE)
>> +	update_flash_db();
>> +#endif
> 
> Is this #ifdef needed? You don't reference ps3flash symbols directly, only by
> opening /dev/ps3flash. If you always call update_flash_db(), you can print an
> error message and the user will notice things haven't been written to flash.


My thinking was that the file I/O code would be removed by the optimizer
when not needed.  I added a message.

  reply	other threads:[~2007-10-09  1:08 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-06 21:35 [patch 0/6] PS3 os area patches for 2.6.24 geoffrey.levand
2007-10-06 21:35 ` [patch 1/6] PS3: Cleanup of os-area.c geoffrey.levand
2007-10-06 21:35 ` [patch 2/6] PS3: Remove unused os-area params geoffrey.levand
2007-10-08 14:00   ` Ranulf Doswell
2007-10-08 17:53     ` Geoff Levand
2007-10-08 22:38       ` Ranulf Doswell
2007-10-08 22:50         ` Geoff Levand
2007-10-06 21:35 ` [patch 3/6] PS3: os-area workqueue processing geoffrey.levand
2007-10-06 21:35 ` [patch 4/6] PS3: Add os-area rtc_diff set/get routines geoffrey.levand
2007-10-06 21:35 ` [patch 5/6] PS3: Save os-area params to device tree geoffrey.levand
2007-10-06 21:35 ` [patch 6/6] PS3: Add os-area database routines geoffrey.levand
2007-10-08  8:27   ` Geert Uytterhoeven
2007-10-09  1:08     ` Geoff Levand [this message]
2007-10-08 12:16   ` Geert Uytterhoeven
2007-10-09  1:12     ` Geoff Levand
2007-10-08 13:48   ` Ranulf Doswell
2007-10-08 17:52     ` Geoff Levand
2007-10-08 22:59       ` Ranulf Doswell
2007-10-08 23:36         ` Geoff Levand
2007-10-09  9:35         ` Geert Uytterhoeven
2007-10-09 12:23           ` Ranulf Doswell
2007-10-09  1:07   ` [patch v2] " Geoff Levand
2007-10-09 11:38     ` Geert Uytterhoeven
2007-10-09 17:15     ` Linas Vepstas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=470AD48B.5060102@am.sony.com \
    --to=geoffrey.levand@am$(echo .)sony.com \
    --cc=Geert.Uytterhoeven@sonycom$(echo .)com \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    --cc=paulus@samba$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox