Re: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

Re: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

[Dave Hansen]

On Tue, 2004-10-12 at 16:29, S, Naveen B wrote: 
> +#define ACPI_MEMORY_DEVICE_HID			"PNP0C80"
> +#define ACPI_MEMORY_DEVICE_DRIVER_NAME		"Hotplug Mem Driver"
> +#define ACPI_MEMORY_DEVICE_NAME			"Hotplug Mem
> Device"
> +
> +#define _COMPONENT		ACPI_MEMORY_DEVICE_COMPONENT

Looks whitespace mangled to me.  Can you try an attachment?

-- Dave



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

[Dave Hansen]

On Tue, 2004-10-12 at 16:29, S, Naveen B wrote:
> Attaching the updated ACPI Memory Driver Patch along with this mail.
> This implements review feedback and is ported to latest kernel
> (2.6.9-rc4-mm1). And this is tested on the real hardware with patchset
> available in http://sprucegoose.sr71.net/patches
> 
> Let me know for any clarifications.

Other than the whitespace problems, this looks pretty sane to me.  Would
you like me to add it to the patch set?  

-- Dave



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

[Dave Hansen]

One more thing, can you give me a short summary of what this patch does,
and maybe what kind of hardware it's useful on for the changelog?

-- Dave



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

[Hiroyuki KAMEZAWA]

as Bjohn said.
> I'm not sure you should assume the range will be the only item in _CRS
> (i.e., you may need to iterate over all items using acpi_walk_resources()).

I think you should use acpi_walk_resources(), like below.

By the way,a _CRS cannot contain plural address information ?

Kame <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>

S, Naveen B wrote:
+static int
+acpi_memory_enable_device(struct acpi_memory_device *mem_device)
+{
+	int result;
+	
+	ACPI_FUNCTION_TRACE("acpi_memory_enable_device");
+
+	/* Get the range from the _CRS */
+	result = acpi_memory_get_device_resources(mem_device);

result = acpi_walk_resources(mem_device->handle, METHOD_NAME_CRS,
                              acpi_memory_get_device_resources, mem_device);

+	if (result) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+			"\nget_device_resources failed\n"));
+		mem_device->state = MEMORY_INVALID_STATE;		
+		return result;
+	}

-- 
--the clue is these footmarks leading to the door.--
KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

RE: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

[S, Naveen B]

> as Bjohn said.
> > I'm not sure you should assume the range will be the only item in
_CRS
> > (i.e., you may need to iterate over all items using
> acpi_walk_resources()).
> 
> I think you should use acpi_walk_resources(), like below.
> 
> By the way,a _CRS cannot contain plural address information ?
> 
> Kame kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org

Thanks Kame,
As I was mentioning earlier, my understanding from the spec that the
memory device has only range in _CRS. So I believe acpi_walk_resources
is redundant in this case. Anyway this could be changed if it is a real
concern.
Please let me know.

--Naveen


> 
> S, Naveen B wrote:
> +static int
> +acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> +{
> +	int result;
> +
> +	ACPI_FUNCTION_TRACE("acpi_memory_enable_device");
> +
> +	/* Get the range from the _CRS */
> +	result = acpi_memory_get_device_resources(mem_device);
> 
> result = acpi_walk_resources(mem_device->handle, METHOD_NAME_CRS,
>                               acpi_memory_get_device_resources,
> mem_device);
> 
> +	if (result) {
> +		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
> +			"\nget_device_resources failed\n"));
> +		mem_device->state = MEMORY_INVALID_STATE;
> +		return result;
> +	}
> 
> --
> --the clue is these footmarks leading to the door.--
> KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

[Hiroyuki KAMEZAWA]

S, Naveen B wrote:

>
> Thanks Kame,
> As I was mentioning earlier, my understanding from the spec that the
> memory device has only range in _CRS. So I believe acpi_walk_resources
> is redundant in this case. Anyway this could be changed if it is a real
> concern.
> Please let me know.
> 
> --Naveen
> 

Okay, thanks.
Currently, I have no *real* concern.

-- Kame <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>


> 
> 
>>S, Naveen B wrote:
>>+static int
>>+acpi_memory_enable_device(struct acpi_memory_device *mem_device)
>>+{
>>+	int result;
>>+
>>+	ACPI_FUNCTION_TRACE("acpi_memory_enable_device");
>>+
>>+	/* Get the range from the _CRS */
>>+	result = acpi_memory_get_device_resources(mem_device);
>>
>>result = acpi_walk_resources(mem_device->handle, METHOD_NAME_CRS,
>>                              acpi_memory_get_device_resources,
>>mem_device);
>>
>>+	if (result) {
>>+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
>>+			"\nget_device_resources failed\n"));
>>+		mem_device->state = MEMORY_INVALID_STATE;
>>+		return result;
>>+	}
>>
>>--
>>--the clue is these footmarks leading to the door.--
>>KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
> 
> 
> 


-- 
--the clue is these footmarks leading to the door.--
KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

RE: [Lhms-devel] [PATCH] ACPI based Memory Hotplug Driver Patch Update

[Dave Hansen]

Your patch contained 63 lines with trailing whitespace.  Please be
careful to strip these before submitting in the future.

-- Dave



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl